[PATCH 00/13] wcn36xx: Enable downstream consistent Wake on Lan

[PATCH 00/13] wcn36xx: Enable downstream consistent Wake on Lan

[Bryan O'Donoghue]

This series enables Wake on WLAN for the wcn36xx. Downstream implements two
competing methods for WoWLAN, only one of which is used in practice.

1. The downstream prima driver and firmware have a dedicated wowlan SMD
   command set, however for whatever reason, the downstream driver doesn't
   actually use that code path for suspend/resume.
   Similarly the downstream kernel will mask out the SMD interrupt when
   entering into suspend, so its clear the firmware assisted wowlan command
   is not used at all.
   In short, the code is implemented but not used.

2. The alternative version of wake on wlan implemented downstream does
   A. An RXP filter
   B. Frimware ipv4 ARP offload
   C. Frimware ipv6 namesapece offload
   D. GTK replay counter save/update on suspend/resume

This patchset replicates method #2 since it is the only method that
actually works. When replicating the functionality from downstream to
upsteam I found the RXP filter step to be redundant, so I skipped that.

Verified on Android/4.19 with qcom out-of-tree low-power patch on wcn3680.
Verified on Debian/kvalo/master on wcn3680 and wcn3620.

The Debian/upstream test doesn't include the low-power patches from qcom so
the time it takes the system to wake up when pinged is longer.

Test method:

1. Enable magic packet wakeup on target
   iw phy0 wowlan enable magic-packet
   iw phy0 wowlan show

2. Flush arp tables on host
   sudo ip -s -s neigh flush all
   sudo ip -6 -s -s neigh flush all

3. Suspend device
   echo 1 > /sys/kernel/debug/clk/debug_suspend
   echo mem > /sys/power/state ; cat /sys/kernel/debug/rpm_stats
   echo 0 > /sys/module/wcn36xx/parameters/debug_mask
   dmesg

4. Ping either ipv4 or ipv6
   ping 192.168.0.85
   ping -6 fe80::6455:44ff:fe33:2211%wlo1

In both cases using Wireshark we see the target system receive an ARP or
NS6 lookup and respond, followed by the host sending a ping or ping6 and
the target system coming out of suspend.

Similarly watching GTK keying on the AP when the device is in suspend, we
see the firmware successfully rekey and when we resume, the host picking up
the rekey material with persistence of the WiFi link during suspend/resume.

commit: 5336fad96e8f ("wcn36xx: Enable firmware offloaded keepalive")
ensures that the link is kept alive in suspend, this additional set gives
the ability to 

- Resolve ARP/NS6 lookups without waking the system
- Rekeying in suspend
- Suspending and resuming based on received datagram

https://git.linaro.org/people/bryan.odonoghue/kernel.git/log/?h=ath.master-wcn36xx-fw-offload-suspend-resume

Bryan O'Donoghue (13):
  wcn36xx: Return result of set_power_params in suspend
  wcn36xx: Run suspend for the first ieee80211_vif
  wcn36xx: Add ipv4 ARP offload support in suspend
  wcn36xx: Do not flush indication queue on suspend/resume
  wcn36xx: Add ipv6 address tracking
  wcn36xx: Add ipv6 namespace offload in suspend
  wcn36xx: Flag WIPHY_WOWLAN_MAGIC_PKT
  wcn36xx: Add set_rekey_data callback
  wcn36xx: Add GTK offload to WoWLAN path
  wcn36xx: Add GTK offload info to WoWLAN resume
  wcn36xx: Do not suspend if scan in progress
  wcn36xx: Add Host suspend indication support
  wcn36xx: Add host resume request support

 drivers/net/wireless/ath/wcn36xx/hal.h     |  20 +-
 drivers/net/wireless/ath/wcn36xx/main.c    | 123 +++++++++-
 drivers/net/wireless/ath/wcn36xx/smd.c     | 271 +++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h     |  19 ++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |  15 ++
 5 files changed, 434 insertions(+), 14 deletions(-)

-- 
2.29.2

[PATCH 01/13] wcn36xx: Return result of set_power_params in suspend

[Bryan O'Donoghue]

wcn36xx_smd_set_power_params() can return an error. For the purposes of
entering into suspend we need the suspend() function to trap and report
errors up the stack.

First step in this process is reporting the existing result code for
wcn36xx_smd_set_power_params().

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 5867bd9c2f64..a863a90232c9 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1091,12 +1091,14 @@ static int wcn36xx_sta_remove(struct ieee80211_hw *hw,
 static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 {
 	struct wcn36xx *wcn = hw->priv;
+	int ret;
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac suspend\n");
 
 	flush_workqueue(wcn->hal_ind_wq);
-	wcn36xx_smd_set_power_params(wcn, true);
-	return 0;
+	ret = wcn36xx_smd_set_power_params(wcn, true);
+
+	return ret;
 }
 
 static int wcn36xx_resume(struct ieee80211_hw *hw)
-- 
2.29.2

[PATCH 02/13] wcn36xx: Run suspend for the first ieee80211_vif

[Bryan O'Donoghue]

A subsequent set of patches will extend out suspend/resume support in this
driver, we cannot set the firmware up for multiple ipv4/ipv6 addresses and
as such we can't iterate through a list of ieee80211_vif.

This patch constrains the interaction with the firmware to the first
ieee80211_vif on the suspend/resume/wowlan path.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 43 +++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index a863a90232c9..b90664bdf6ee 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1088,15 +1088,39 @@ static int wcn36xx_sta_remove(struct ieee80211_hw *hw,
 
 #ifdef CONFIG_PM
 
+struct ieee80211_vif *wcn36xx_get_first_vif(struct wcn36xx *wcn)
+{
+	struct wcn36xx_vif *tmp;
+	struct ieee80211_vif *vif = NULL;
+
+	list_for_each_entry(tmp, &wcn->vif_list, list) {
+		vif = wcn36xx_priv_to_vif(tmp);
+		if (vif)
+			break;
+	}
+	return vif;
+}
+
 static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 {
 	struct wcn36xx *wcn = hw->priv;
-	int ret;
+	struct ieee80211_vif *vif = NULL;
+	struct wcn36xx_vif *vif_priv = NULL;
+	int ret = 0;
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac suspend\n");
 
 	flush_workqueue(wcn->hal_ind_wq);
-	ret = wcn36xx_smd_set_power_params(wcn, true);
+	mutex_lock(&wcn->conf_mutex);
+	vif = wcn36xx_get_first_vif(wcn);
+	if (vif) {
+		vif_priv = wcn36xx_vif_to_priv(vif);
+		if (!vif_priv->sta_assoc)
+			goto out;
+		ret = wcn36xx_smd_set_power_params(wcn, true);
+	}
+out:
+	mutex_unlock(&wcn->conf_mutex);
 
 	return ret;
 }
@@ -1104,11 +1128,24 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 static int wcn36xx_resume(struct ieee80211_hw *hw)
 {
 	struct wcn36xx *wcn = hw->priv;
+	struct ieee80211_vif *vif = NULL;
+	struct wcn36xx_vif *vif_priv = NULL;
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac resume\n");
 
 	flush_workqueue(wcn->hal_ind_wq);
-	wcn36xx_smd_set_power_params(wcn, false);
+	mutex_lock(&wcn->conf_mutex);
+	vif = wcn36xx_get_first_vif(wcn);
+	if (vif) {
+		vif_priv = wcn36xx_vif_to_priv(vif);
+		if (!vif_priv->sta_assoc)
+			goto out;
+
+		wcn36xx_smd_set_power_params(wcn, false);
+	}
+out:
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
-- 
2.29.2

Re: [PATCH 02/13] wcn36xx: Run suspend for the first ieee80211_vif

[kernel test robot]

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

Hi Bryan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master ath6kl/ath-next v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bryan-O-Donoghue/wcn36xx-Enable-downstream-consistent-Wake-on-Lan/20201229-003134
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/76ccb1ff883813f6950d31c6c82c47a180682c4c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bryan-O-Donoghue/wcn36xx-Enable-downstream-consistent-Wake-on-Lan/20201229-003134
        git checkout 76ccb1ff883813f6950d31c6c82c47a180682c4c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/wireless/ath/wcn36xx/main.c:1091:23: warning: no previous prototype for 'wcn36xx_get_first_vif' [-Wmissing-prototypes]
    1091 | struct ieee80211_vif *wcn36xx_get_first_vif(struct wcn36xx *wcn)
         |                       ^~~~~~~~~~~~~~~~~~~~~


vim +/wcn36xx_get_first_vif +1091 drivers/net/wireless/ath/wcn36xx/main.c

  1090	
> 1091	struct ieee80211_vif *wcn36xx_get_first_vif(struct wcn36xx *wcn)
  1092	{
  1093		struct wcn36xx_vif *tmp;
  1094		struct ieee80211_vif *vif = NULL;
  1095	
  1096		list_for_each_entry(tmp, &wcn->vif_list, list) {
  1097			vif = wcn36xx_priv_to_vif(tmp);
  1098			if (vif)
  1099				break;
  1100		}
  1101		return vif;
  1102	}
  1103	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66525 bytes --]

[PATCH 03/13] wcn36xx: Add ipv4 ARP offload support in suspend

[Bryan O'Donoghue]

This commit adds ARP offload support. Firmware is capable of responding to
ARP requests for a single ipv4 address only.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h  | 10 +++----
 drivers/net/wireless/ath/wcn36xx/main.c |  5 +++-
 drivers/net/wireless/ath/wcn36xx/smd.c  | 38 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  4 +++
 4 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 65ef893f2736..b56c8292fa62 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -3465,7 +3465,7 @@ struct wcn36xx_hal_rem_bcn_filter_req {
 #define WCN36XX_HAL_OFFLOAD_ENABLE                          1
 #define WCN36XX_HAL_OFFLOAD_BCAST_FILTER_ENABLE             0x2
 #define WCN36XX_HAL_OFFLOAD_ARP_AND_BCAST_FILTER_ENABLE	\
-	(HAL_OFFLOAD_ENABLE|HAL_OFFLOAD_BCAST_FILTER_ENABLE)
+	(WCN36XX_HAL_OFFLOAD_ENABLE | WCN36XX_HAL_OFFLOAD_BCAST_FILTER_ENABLE)
 
 struct wcn36xx_hal_ns_offload_params {
 	u8 src_ipv6_addr[WCN36XX_HAL_IPV6_ADDR_LEN];
@@ -3487,10 +3487,10 @@ struct wcn36xx_hal_ns_offload_params {
 	/* slot index for this offload */
 	u32 slot_index;
 	u8 bss_index;
-};
+} __packed;
 
 struct wcn36xx_hal_host_offload_req {
-	u8 offload_Type;
+	u8 offload_type;
 
 	/* enable or disable */
 	u8 enable;
@@ -3499,13 +3499,13 @@ struct wcn36xx_hal_host_offload_req {
 		u8 host_ipv4_addr[4];
 		u8 host_ipv6_addr[WCN36XX_HAL_IPV6_ADDR_LEN];
 	} u;
-};
+} __packed;
 
 struct wcn36xx_hal_host_offload_req_msg {
 	struct wcn36xx_hal_msg_header header;
 	struct wcn36xx_hal_host_offload_req host_offload_params;
 	struct wcn36xx_hal_ns_offload_params ns_offload_params;
-};
+} __packed;
 
 /* Packet Types. */
 #define WCN36XX_HAL_KEEP_ALIVE_NULL_PKT              1
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index b90664bdf6ee..c463ec7c1568 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1117,11 +1117,13 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 		vif_priv = wcn36xx_vif_to_priv(vif);
 		if (!vif_priv->sta_assoc)
 			goto out;
+		ret = wcn36xx_smd_arp_offload(wcn, vif, true);
+		if (ret)
+			goto out;
 		ret = wcn36xx_smd_set_power_params(wcn, true);
 	}
 out:
 	mutex_unlock(&wcn->conf_mutex);
-
 	return ret;
 }
 
@@ -1142,6 +1144,7 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 			goto out;
 
 		wcn36xx_smd_set_power_params(wcn, false);
+		wcn36xx_smd_arp_offload(wcn, vif, false);
 	}
 out:
 	mutex_unlock(&wcn->conf_mutex);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 5445277dd8de..4589c3d26027 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2757,6 +2757,43 @@ int wcn36xx_smd_set_mc_list(struct wcn36xx *wcn,
 	return ret;
 }
 
+int wcn36xx_smd_arp_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
+			    bool enable)
+{
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	struct wcn36xx_hal_host_offload_req_msg msg_body;
+	int ret;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_HOST_OFFLOAD_REQ);
+	msg_body.host_offload_params.offload_type =
+		WCN36XX_HAL_IPV4_ARP_REPLY_OFFLOAD;
+	if (enable) {
+		msg_body.host_offload_params.enable =
+			WCN36XX_HAL_OFFLOAD_ARP_AND_BCAST_FILTER_ENABLE;
+		memcpy(&msg_body.host_offload_params.u,
+		       &vif->bss_conf.arp_addr_list[0], sizeof(__be32));
+	}
+	msg_body.ns_offload_params.bss_index = vif_priv->bss_index;
+
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
+	if (ret) {
+		wcn36xx_err("Sending host_offload_arp failed\n");
+		goto out;
+	}
+	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	if (ret) {
+		wcn36xx_err("host_offload_arp failed err=%d\n", ret);
+		goto out;
+	}
+out:
+	mutex_unlock(&wcn->hal_mutex);
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
@@ -2805,6 +2842,7 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 	case WCN36XX_HAL_8023_MULTICAST_LIST_RSP:
 	case WCN36XX_HAL_START_SCAN_OFFLOAD_RSP:
 	case WCN36XX_HAL_STOP_SCAN_OFFLOAD_RSP:
+	case WCN36XX_HAL_HOST_OFFLOAD_RSP:
 		memcpy(wcn->hal_buf, buf, len);
 		wcn->hal_rsp_len = len;
 		complete(&wcn->hal_rsp_compl);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index b1d8083d9d9d..1458682ee715 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -146,4 +146,8 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 int wcn36xx_smd_set_mc_list(struct wcn36xx *wcn,
 			    struct ieee80211_vif *vif,
 			    struct wcn36xx_hal_rcv_flt_mc_addr_list_type *fp);
+
+int wcn36xx_smd_arp_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
+			    bool enable);
+
 #endif	/* _SMD_H_ */
-- 
2.29.2

Re: [PATCH 03/13] wcn36xx: Add ipv4 ARP offload support in suspend

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> This commit adds ARP offload support. Firmware is capable of responding to
> ARP requests for a single ipv4 address only.

This is nitpicking, but you don't need to say "This commit..." or "This
patch...", instead you can simply say: "Add ARP offload support...".

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

[PATCH 04/13] wcn36xx: Do not flush indication queue on suspend/resume

[Bryan O'Donoghue]

Testing on Android reveals that the flush on both suspend and resume of the
firmware indication work-queue can stall indefinitely.

Given this code path doesn't appear to have been exercised up until now,
removing this flush to unblock this situation.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index c463ec7c1568..745512a6ad83 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1110,7 +1110,6 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac suspend\n");
 
-	flush_workqueue(wcn->hal_ind_wq);
 	mutex_lock(&wcn->conf_mutex);
 	vif = wcn36xx_get_first_vif(wcn);
 	if (vif) {
@@ -1135,7 +1134,6 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac resume\n");
 
-	flush_workqueue(wcn->hal_ind_wq);
 	mutex_lock(&wcn->conf_mutex);
 	vif = wcn36xx_get_first_vif(wcn);
 	if (vif) {
-- 
2.29.2

[PATCH 05/13] wcn36xx: Add ipv6 address tracking

[Bryan O'Donoghue]

Taking code from iwlwifi this commit adds a standard callback for
ipv6_addr_change().

This callback allows wcn36xx to know the set of ipv6 addresses. Something
we need to know in order to get wowlan working with ipv6.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h     |  1 +
 drivers/net/wireless/ath/wcn36xx/main.c    | 32 ++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |  8 ++++++
 3 files changed, 41 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index b56c8292fa62..90333daed845 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -3466,6 +3466,7 @@ struct wcn36xx_hal_rem_bcn_filter_req {
 #define WCN36XX_HAL_OFFLOAD_BCAST_FILTER_ENABLE             0x2
 #define WCN36XX_HAL_OFFLOAD_ARP_AND_BCAST_FILTER_ENABLE	\
 	(WCN36XX_HAL_OFFLOAD_ENABLE | WCN36XX_HAL_OFFLOAD_BCAST_FILTER_ENABLE)
+#define WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX		0x02
 
 struct wcn36xx_hal_ns_offload_params {
 	u8 src_ipv6_addr[WCN36XX_HAL_IPV6_ADDR_LEN];
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 745512a6ad83..af59638d696f 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -25,6 +25,7 @@
 #include <linux/rpmsg.h>
 #include <linux/soc/qcom/smem_state.h>
 #include <linux/soc/qcom/wcnss_ctrl.h>
+#include <net/ipv6.h>
 #include "wcn36xx.h"
 #include "testmode.h"
 
@@ -1215,6 +1216,34 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw,
 	return ret;
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+void wcn36xx_ipv6_addr_change(struct ieee80211_hw *hw,
+			      struct ieee80211_vif *vif,
+			      struct inet6_dev *idev)
+{
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	struct inet6_ifaddr *ifa;
+	int idx = 0;
+
+	memset(vif_priv->tentative_addrs, 0, sizeof(vif_priv->tentative_addrs));
+
+	read_lock_bh(&idev->lock);
+	list_for_each_entry(ifa, &idev->addr_list, if_list) {
+		vif_priv->target_ipv6_addrs[idx] = ifa->addr;
+		if (ifa->flags & IFA_F_TENTATIVE)
+			__set_bit(idx, vif_priv->tentative_addrs);
+		idx++;
+		if (idx >= WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX)
+			break;
+		wcn36xx_dbg(WCN36XX_DBG_MAC, "%pI6 %s\n", &ifa->addr,
+			    (ifa->flags & IFA_F_TENTATIVE) ? "tentative" : NULL);
+	}
+	read_unlock_bh(&idev->lock);
+
+	vif_priv->num_target_ipv6_addrs = idx;
+}
+#endif
+
 static const struct ieee80211_ops wcn36xx_ops = {
 	.start			= wcn36xx_start,
 	.stop			= wcn36xx_stop,
@@ -1238,6 +1267,9 @@ static const struct ieee80211_ops wcn36xx_ops = {
 	.sta_add		= wcn36xx_sta_add,
 	.sta_remove		= wcn36xx_sta_remove,
 	.ampdu_action		= wcn36xx_ampdu_action,
+#if IS_ENABLED(CONFIG_IPV6)
+	.ipv6_addr_change	= wcn36xx_ipv6_addr_change,
+#endif
 
 	CFG80211_TESTMODE_CMD(wcn36xx_tm_cmd)
 };
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 71fa9992b118..5a5114660b18 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -18,6 +18,7 @@
 #define _WCN36XX_H_
 
 #include <linux/completion.h>
+#include <linux/in6.h>
 #include <linux/printk.h>
 #include <linux/spinlock.h>
 #include <net/mac80211.h>
@@ -136,6 +137,13 @@ struct wcn36xx_vif {
 	u8 self_dpu_desc_index;
 	u8 self_ucast_dpu_sign;
 
+#if IS_ENABLED(CONFIG_IPV6)
+	/* IPv6 addresses for WoWLAN */
+	struct in6_addr target_ipv6_addrs[WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX];
+	unsigned long tentative_addrs[BITS_TO_LONGS(WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX)];
+	int num_target_ipv6_addrs;
+#endif
+
 	struct list_head sta_list;
 };
 
-- 
2.29.2

Re: [PATCH 05/13] wcn36xx: Add ipv6 address tracking

[kernel test robot]

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

Hi Bryan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master ath6kl/ath-next v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bryan-O-Donoghue/wcn36xx-Enable-downstream-consistent-Wake-on-Lan/20201229-003134
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a7d3b150ccc65a022e531e7648080be9fedb1318
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bryan-O-Donoghue/wcn36xx-Enable-downstream-consistent-Wake-on-Lan/20201229-003134
        git checkout a7d3b150ccc65a022e531e7648080be9fedb1318
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/wireless/ath/wcn36xx/main.c:1092:23: warning: no previous prototype for 'wcn36xx_get_first_vif' [-Wmissing-prototypes]
    1092 | struct ieee80211_vif *wcn36xx_get_first_vif(struct wcn36xx *wcn)
         |                       ^~~~~~~~~~~~~~~~~~~~~
>> drivers/net/wireless/ath/wcn36xx/main.c:1220:6: warning: no previous prototype for 'wcn36xx_ipv6_addr_change' [-Wmissing-prototypes]
    1220 | void wcn36xx_ipv6_addr_change(struct ieee80211_hw *hw,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/wcn36xx_ipv6_addr_change +1220 drivers/net/wireless/ath/wcn36xx/main.c

  1218	
  1219	#if IS_ENABLED(CONFIG_IPV6)
> 1220	void wcn36xx_ipv6_addr_change(struct ieee80211_hw *hw,
  1221				      struct ieee80211_vif *vif,
  1222				      struct inet6_dev *idev)
  1223	{
  1224		struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
  1225		struct inet6_ifaddr *ifa;
  1226		int idx = 0;
  1227	
  1228		memset(vif_priv->tentative_addrs, 0, sizeof(vif_priv->tentative_addrs));
  1229	
  1230		read_lock_bh(&idev->lock);
  1231		list_for_each_entry(ifa, &idev->addr_list, if_list) {
  1232			vif_priv->target_ipv6_addrs[idx] = ifa->addr;
  1233			if (ifa->flags & IFA_F_TENTATIVE)
  1234				__set_bit(idx, vif_priv->tentative_addrs);
  1235			idx++;
  1236			if (idx >= WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX)
  1237				break;
  1238			wcn36xx_dbg(WCN36XX_DBG_MAC, "%pI6 %s\n", &ifa->addr,
  1239				    (ifa->flags & IFA_F_TENTATIVE) ? "tentative" : NULL);
  1240		}
  1241		read_unlock_bh(&idev->lock);
  1242	
  1243		vif_priv->num_target_ipv6_addrs = idx;
  1244	}
  1245	#endif
  1246	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66525 bytes --]

[PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Bryan O'Donoghue]

We need to respond to ipv6 namespace lookups when in suspend. This patch
adds the necessary changes to issue the appropriate firmware command on
suspend and resume to enter/exit firmware offloaded ns lookup.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h  |  3 ++
 drivers/net/wireless/ath/wcn36xx/main.c |  4 ++
 drivers/net/wireless/ath/wcn36xx/smd.c  | 55 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  3 ++
 4 files changed, 65 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 90333daed845..3b949b0b3792 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -3464,6 +3464,9 @@ struct wcn36xx_hal_rem_bcn_filter_req {
 #define WCN36XX_HAL_OFFLOAD_DISABLE                         0
 #define WCN36XX_HAL_OFFLOAD_ENABLE                          1
 #define WCN36XX_HAL_OFFLOAD_BCAST_FILTER_ENABLE             0x2
+#define WCN36XX_HAL_OFFLOAD_MCAST_FILTER_ENABLE             0x4
+#define WCN36XX_HAL_OFFLOAD_NS_AND_MCAST_FILTER_ENABLE	\
+	(WCN36XX_HAL_OFFLOAD_ENABLE | WCN36XX_HAL_OFFLOAD_MCAST_FILTER_ENABLE)
 #define WCN36XX_HAL_OFFLOAD_ARP_AND_BCAST_FILTER_ENABLE	\
 	(WCN36XX_HAL_OFFLOAD_ENABLE | WCN36XX_HAL_OFFLOAD_BCAST_FILTER_ENABLE)
 #define WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX		0x02
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index af59638d696f..579bb7f4db9c 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1118,6 +1118,9 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 		if (!vif_priv->sta_assoc)
 			goto out;
 		ret = wcn36xx_smd_arp_offload(wcn, vif, true);
+		if (ret)
+			goto out;
+		ret = wcn36xx_smd_ipv6_ns_offload(wcn, vif, true);
 		if (ret)
 			goto out;
 		ret = wcn36xx_smd_set_power_params(wcn, true);
@@ -1143,6 +1146,7 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 			goto out;
 
 		wcn36xx_smd_set_power_params(wcn, false);
+		wcn36xx_smd_ipv6_ns_offload(wcn, vif, false);
 		wcn36xx_smd_arp_offload(wcn, vif, false);
 	}
 out:
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 4589c3d26027..b05f1e1f253a 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2794,6 +2794,61 @@ int wcn36xx_smd_arp_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 	return ret;
 }
 
+int wcn36xx_smd_ipv6_ns_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
+				bool enable)
+{
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	struct wcn36xx_hal_host_offload_req_msg msg_body;
+	struct wcn36xx_hal_ns_offload_params *ns_params;
+	struct wcn36xx_hal_host_offload_req *ho_params;
+	int ret;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_HOST_OFFLOAD_REQ);
+	ho_params = &msg_body.host_offload_params;
+	ns_params = &msg_body.ns_offload_params;
+
+	ho_params->offload_type = WCN36XX_HAL_IPV6_NS_OFFLOAD;
+	if (enable) {
+		ho_params->enable =
+			WCN36XX_HAL_OFFLOAD_NS_AND_MCAST_FILTER_ENABLE;
+		if (vif_priv->num_target_ipv6_addrs) {
+			memcpy(&ho_params->u,
+			       &vif_priv->target_ipv6_addrs[0].in6_u,
+			       sizeof(struct in6_addr));
+			memcpy(&ns_params->target_ipv6_addr1,
+			       &vif_priv->target_ipv6_addrs[0].in6_u,
+			       sizeof(struct in6_addr));
+			ns_params->target_ipv6_addr1_valid = 1;
+		}
+		if (vif_priv->num_target_ipv6_addrs > 1) {
+			memcpy(&ns_params->target_ipv6_addr2,
+			       &vif_priv->target_ipv6_addrs[1].in6_u,
+			       sizeof(struct in6_addr));
+			ns_params->target_ipv6_addr2_valid = 1;
+		}
+	}
+	memcpy(&ns_params->self_addr, vif->addr, ETH_ALEN);
+	ns_params->bss_index = vif_priv->bss_index;
+
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
+	if (ret) {
+		wcn36xx_err("Sending host_offload_arp failed\n");
+		goto out;
+	}
+	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	if (ret) {
+		wcn36xx_err("host_offload_arp failed err=%d\n", ret);
+		goto out;
+	}
+out:
+	mutex_unlock(&wcn->hal_mutex);
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 1458682ee715..380ea58b58ec 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -150,4 +150,7 @@ int wcn36xx_smd_set_mc_list(struct wcn36xx *wcn,
 int wcn36xx_smd_arp_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 			    bool enable);
 
+int wcn36xx_smd_ipv6_ns_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
+				bool enable);
+
 #endif	/* _SMD_H_ */
-- 
2.29.2

Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[kernel test robot]

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

Hi Bryan,

I love your patch! Yet something to improve:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on wireless-drivers/master ath6kl/ath-next v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bryan-O-Donoghue/wcn36xx-Enable-downstream-consistent-Wake-on-Lan/20201229-003134
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: m68k-randconfig-r004-20201228 (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3af46faf5418cb35d31847e75156f9bb24b3828a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bryan-O-Donoghue/wcn36xx-Enable-downstream-consistent-Wake-on-Lan/20201229-003134
        git checkout 3af46faf5418cb35d31847e75156f9bb24b3828a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:11,
                    from include/linux/skbuff.h:13,
                    from include/linux/if_ether.h:19,
                    from include/linux/etherdevice.h:20,
                    from drivers/net/wireless/ath/wcn36xx/smd.c:19:
   include/linux/scatterlist.h: In function 'sg_set_buf':
   arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of pointer with null pointer [-Wextra]
     169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void *)PAGE_OFFSET && (void *)(kaddr) < high_memory)
         |                                                 ^~
   include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
      78 | # define unlikely(x) __builtin_expect(!!(x), 0)
         |                                          ^
   include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
     143 |  BUG_ON(!virt_addr_valid(buf));
         |  ^~~~~~
   include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
     143 |  BUG_ON(!virt_addr_valid(buf));
         |          ^~~~~~~~~~~~~~~
   drivers/net/wireless/ath/wcn36xx/smd.c: In function 'wcn36xx_smd_ipv6_ns_offload':
>> drivers/net/wireless/ath/wcn36xx/smd.c:2816:15: error: 'struct wcn36xx_vif' has no member named 'num_target_ipv6_addrs'
    2816 |   if (vif_priv->num_target_ipv6_addrs) {
         |               ^~
   In file included from include/linux/string.h:20,
                    from include/linux/bitmap.h:9,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:59,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:6,
                    from include/linux/mm.h:10,
                    from include/linux/bvec.h:14,
                    from include/linux/skbuff.h:17,
                    from include/linux/if_ether.h:19,
                    from include/linux/etherdevice.h:20,
                    from drivers/net/wireless/ath/wcn36xx/smd.c:19:
>> drivers/net/wireless/ath/wcn36xx/smd.c:2818:20: error: 'struct wcn36xx_vif' has no member named 'target_ipv6_addrs'
    2818 |           &vif_priv->target_ipv6_addrs[0].in6_u,
         |                    ^~
   arch/m68k/include/asm/string.h:72:45: note: in definition of macro 'memcpy'
      72 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
         |                                             ^
   drivers/net/wireless/ath/wcn36xx/smd.c:2821:20: error: 'struct wcn36xx_vif' has no member named 'target_ipv6_addrs'
    2821 |           &vif_priv->target_ipv6_addrs[0].in6_u,
         |                    ^~
   arch/m68k/include/asm/string.h:72:45: note: in definition of macro 'memcpy'
      72 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
         |                                             ^
   drivers/net/wireless/ath/wcn36xx/smd.c:2825:15: error: 'struct wcn36xx_vif' has no member named 'num_target_ipv6_addrs'
    2825 |   if (vif_priv->num_target_ipv6_addrs > 1) {
         |               ^~
   In file included from include/linux/string.h:20,
                    from include/linux/bitmap.h:9,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:59,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:6,
                    from include/linux/mm.h:10,
                    from include/linux/bvec.h:14,
                    from include/linux/skbuff.h:17,
                    from include/linux/if_ether.h:19,
                    from include/linux/etherdevice.h:20,
                    from drivers/net/wireless/ath/wcn36xx/smd.c:19:
   drivers/net/wireless/ath/wcn36xx/smd.c:2827:20: error: 'struct wcn36xx_vif' has no member named 'target_ipv6_addrs'
    2827 |           &vif_priv->target_ipv6_addrs[1].in6_u,
         |                    ^~
   arch/m68k/include/asm/string.h:72:45: note: in definition of macro 'memcpy'
      72 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
         |                                             ^


vim +2816 drivers/net/wireless/ath/wcn36xx/smd.c

  2796	
  2797	int wcn36xx_smd_ipv6_ns_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
  2798					bool enable)
  2799	{
  2800		struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
  2801		struct wcn36xx_hal_host_offload_req_msg msg_body;
  2802		struct wcn36xx_hal_ns_offload_params *ns_params;
  2803		struct wcn36xx_hal_host_offload_req *ho_params;
  2804		int ret;
  2805	
  2806		mutex_lock(&wcn->hal_mutex);
  2807	
  2808		INIT_HAL_MSG(msg_body, WCN36XX_HAL_HOST_OFFLOAD_REQ);
  2809		ho_params = &msg_body.host_offload_params;
  2810		ns_params = &msg_body.ns_offload_params;
  2811	
  2812		ho_params->offload_type = WCN36XX_HAL_IPV6_NS_OFFLOAD;
  2813		if (enable) {
  2814			ho_params->enable =
  2815				WCN36XX_HAL_OFFLOAD_NS_AND_MCAST_FILTER_ENABLE;
> 2816			if (vif_priv->num_target_ipv6_addrs) {
  2817				memcpy(&ho_params->u,
> 2818				       &vif_priv->target_ipv6_addrs[0].in6_u,
  2819				       sizeof(struct in6_addr));
  2820				memcpy(&ns_params->target_ipv6_addr1,
  2821				       &vif_priv->target_ipv6_addrs[0].in6_u,
  2822				       sizeof(struct in6_addr));
  2823				ns_params->target_ipv6_addr1_valid = 1;
  2824			}
  2825			if (vif_priv->num_target_ipv6_addrs > 1) {
  2826				memcpy(&ns_params->target_ipv6_addr2,
  2827				       &vif_priv->target_ipv6_addrs[1].in6_u,
  2828				       sizeof(struct in6_addr));
  2829				ns_params->target_ipv6_addr2_valid = 1;
  2830			}
  2831		}
  2832		memcpy(&ns_params->self_addr, vif->addr, ETH_ALEN);
  2833		ns_params->bss_index = vif_priv->bss_index;
  2834	
  2835		PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
  2836	
  2837		ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
  2838		if (ret) {
  2839			wcn36xx_err("Sending host_offload_arp failed\n");
  2840			goto out;
  2841		}
  2842		ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
  2843		if (ret) {
  2844			wcn36xx_err("host_offload_arp failed err=%d\n", ret);
  2845			goto out;
  2846		}
  2847	out:
  2848		mutex_unlock(&wcn->hal_mutex);
  2849		return ret;
  2850	}
  2851	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23005 bytes --]

Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Bryan O'Donoghue]

On 28/12/2020 21:49, kernel test robot wrote:
> [auto build test ERROR on wireless-drivers-next/master]

hmm

works against this

* 77da2c99eca0 - (tag: ath-202012180905, ath.git/master) Add 
localversion-wireless-testing-ath (11 days ago)

Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> On 28/12/2020 21:49, kernel test robot wrote:
>> [auto build test ERROR on wireless-drivers-next/master]
>
> hmm
>
> works against this
>
> * 77da2c99eca0 - (tag: ath-202012180905, ath.git/master) Add
> localversion-wireless-testing-ath (11 days ago)

The bot tested only ath-next from ath.git, not the master branch:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on wireless-drivers/master ath6kl/ath-next v5.11-rc1 next-20201223]

I'm guessing that this build error is due to some API changes in
net-next. I'm planning to update ath.git tree today to v5.11, hopefully
that clears up the issue.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [kbuild-all] Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Chen, Rong A]

On 1/11/2021 7:27 PM, Kalle Valo wrote:
> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
> 
>> On 28/12/2020 21:49, kernel test robot wrote:
>>> [auto build test ERROR on wireless-drivers-next/master]
>>
>> hmm
>>
>> works against this
>>
>> * 77da2c99eca0 - (tag: ath-202012180905, ath.git/master) Add
>> localversion-wireless-testing-ath (11 days ago)
> 
> The bot tested only ath-next from ath.git, not the master branch:

Hi,

Thanks for the help, we have switched to test on master branch.

Best Regards,
Rong Chen

> 
> [auto build test ERROR on wireless-drivers-next/master]
> [also build test ERROR on wireless-drivers/master ath6kl/ath-next v5.11-rc1 next-20201223]
> 
> I'm guessing that this build error is due to some API changes in
> net-next. I'm planning to update ath.git tree today to v5.11, hopefully
> that clears up the issue.
> 

Re: [kbuild-all] Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Kalle Valo]

"Chen, Rong A" <rong.a.chen@intel.com> writes:

> On 1/11/2021 7:27 PM, Kalle Valo wrote:
>> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
>>
>>> On 28/12/2020 21:49, kernel test robot wrote:
>>>> [auto build test ERROR on wireless-drivers-next/master]
>>>
>>> hmm
>>>
>>> works against this
>>>
>>> * 77da2c99eca0 - (tag: ath-202012180905, ath.git/master) Add
>>> localversion-wireless-testing-ath (11 days ago)
>>
>> The bot tested only ath-next from ath.git, not the master branch:
>
> Hi,
>
> Thanks for the help, we have switched to test on master branch.

The bot was correctly testing the ath-next branch, as I use that branch
to commit the patches. Can you still change the bot back to use
ath-next, just as it did previously?

Sorry for the confusion.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [kbuild-all] Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Rong Chen]

On 1/12/21 7:18 PM, Kalle Valo wrote:
> "Chen, Rong A" <rong.a.chen@intel.com> writes:
>
>> On 1/11/2021 7:27 PM, Kalle Valo wrote:
>>> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
>>>
>>>> On 28/12/2020 21:49, kernel test robot wrote:
>>>>> [auto build test ERROR on wireless-drivers-next/master]
>>>> hmm
>>>>
>>>> works against this
>>>>
>>>> * 77da2c99eca0 - (tag: ath-202012180905, ath.git/master) Add
>>>> localversion-wireless-testing-ath (11 days ago)
>>> The bot tested only ath-next from ath.git, not the master branch:
>> Hi,
>>
>> Thanks for the help, we have switched to test on master branch.
> The bot was correctly testing the ath-next branch, as I use that branch
> to commit the patches. Can you still change the bot back to use
> ath-next, just as it did previously?
>
> Sorry for the confusion.
>

Got it, we'll still test ath-next branch.

Best Regards,
Rong Chen

Re: [kbuild-all] Re: [PATCH 06/13] wcn36xx: Add ipv6 namespace offload in suspend

[Kalle Valo]

Rong Chen <rong.a.chen@intel.com> writes:

> On 1/12/21 7:18 PM, Kalle Valo wrote:
>> "Chen, Rong A" <rong.a.chen@intel.com> writes:
>>
>>> On 1/11/2021 7:27 PM, Kalle Valo wrote:
>>>> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
>>>>
>>>>> On 28/12/2020 21:49, kernel test robot wrote:
>>>>>> [auto build test ERROR on wireless-drivers-next/master]
>>>>> hmm
>>>>>
>>>>> works against this
>>>>>
>>>>> * 77da2c99eca0 - (tag: ath-202012180905, ath.git/master) Add
>>>>> localversion-wireless-testing-ath (11 days ago)
>>>> The bot tested only ath-next from ath.git, not the master branch:
>>> Hi,
>>>
>>> Thanks for the help, we have switched to test on master branch.
>> The bot was correctly testing the ath-next branch, as I use that branch
>> to commit the patches. Can you still change the bot back to use
>> ath-next, just as it did previously?
>>
>> Sorry for the confusion.
>>
>
> Got it, we'll still test ath-next branch.

Great, thank you. And thank you for maintaing kbuild bot, it's awesome :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

[PATCH 07/13] wcn36xx: Flag WIPHY_WOWLAN_MAGIC_PKT

[Bryan O'Donoghue]

Set a flag to indicate magic packet wakeup support on wcn36xx.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 579bb7f4db9c..cfa2e2d20a20 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -173,7 +173,7 @@ static struct ieee80211_supported_band wcn_band_5ghz = {
 #ifdef CONFIG_PM
 
 static const struct wiphy_wowlan_support wowlan_support = {
-	.flags = WIPHY_WOWLAN_ANY
+	.flags = WIPHY_WOWLAN_ANY | WIPHY_WOWLAN_MAGIC_PKT
 };
 
 #endif
-- 
2.29.2

Re: [PATCH 07/13] wcn36xx: Flag WIPHY_WOWLAN_MAGIC_PKT

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Set a flag to indicate magic packet wakeup support on wcn36xx.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c
> b/drivers/net/wireless/ath/wcn36xx/main.c
> index 579bb7f4db9c..cfa2e2d20a20 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -173,7 +173,7 @@ static struct ieee80211_supported_band wcn_band_5ghz = {
>  #ifdef CONFIG_PM
>  
>  static const struct wiphy_wowlan_support wowlan_support = {
> -	.flags = WIPHY_WOWLAN_ANY
> +	.flags = WIPHY_WOWLAN_ANY | WIPHY_WOWLAN_MAGIC_PKT
>  };

Shouldn't this be the last patch so that we don't enable the feature
until it's fully implemented? This makes a difference when doing a
bisect, for example.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 07/13] wcn36xx: Flag WIPHY_WOWLAN_MAGIC_PKT

[Bryan O'Donoghue]

On 11/01/2021 11:38, Kalle Valo wrote:
> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
> 
>> Set a flag to indicate magic packet wakeup support on wcn36xx.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c
>> b/drivers/net/wireless/ath/wcn36xx/main.c
>> index 579bb7f4db9c..cfa2e2d20a20 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -173,7 +173,7 @@ static struct ieee80211_supported_band wcn_band_5ghz = {
>>   #ifdef CONFIG_PM
>>   
>>   static const struct wiphy_wowlan_support wowlan_support = {
>> -	.flags = WIPHY_WOWLAN_ANY
>> +	.flags = WIPHY_WOWLAN_ANY | WIPHY_WOWLAN_MAGIC_PKT
>>   };
> 
> Shouldn't this be the last patch so that we don't enable the feature
> until it's fully implemented? This makes a difference when doing a
> bisect, for example.
> 

Sure, I'll add the flags from the subsequent patch and stack on-top so 
the enable is the last patch for magic packet and gtk rekey

[PATCH 08/13] wcn36xx: Add set_rekey_data callback

[Bryan O'Donoghue]

This commit adds a callback for Group Temporal Key tracking as provided by
the standard WiFi ops structure.

We track the key to integrate GTK offloading into the WoWLAN suspend path
later on. Code comes from the Intel iwlwifi driver with minimal name
changes.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c    | 19 +++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |  6 ++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index cfa2e2d20a20..b3388e4a76d2 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1155,6 +1155,24 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 	return 0;
 }
 
+void wcn36xx_set_rekey_data(struct ieee80211_hw *hw,
+			    struct ieee80211_vif *vif,
+			    struct cfg80211_gtk_rekey_data *data)
+{
+	struct wcn36xx *wcn = hw->priv;
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+
+	mutex_lock(&wcn->conf_mutex);
+
+	memcpy(vif_priv->rekey_data.kek, data->kek, NL80211_KEK_LEN);
+	memcpy(vif_priv->rekey_data.kck, data->kck, NL80211_KCK_LEN);
+	vif_priv->rekey_data.replay_ctr =
+		cpu_to_le64(be64_to_cpup((__be64 *)data->replay_ctr));
+	vif_priv->rekey_data.valid = true;
+
+	mutex_unlock(&wcn->conf_mutex);
+}
+
 #endif
 
 static int wcn36xx_ampdu_action(struct ieee80211_hw *hw,
@@ -1256,6 +1274,7 @@ static const struct ieee80211_ops wcn36xx_ops = {
 #ifdef CONFIG_PM
 	.suspend		= wcn36xx_suspend,
 	.resume			= wcn36xx_resume,
+	.set_rekey_data		= wcn36xx_set_rekey_data,
 #endif
 	.config			= wcn36xx_config,
 	.prepare_multicast	= wcn36xx_prepare_multicast,
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 5a5114660b18..6121d8a5641a 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -143,6 +143,12 @@ struct wcn36xx_vif {
 	unsigned long tentative_addrs[BITS_TO_LONGS(WCN36XX_HAL_IPV6_OFFLOAD_ADDR_MAX)];
 	int num_target_ipv6_addrs;
 #endif
+	/* WoWLAN GTK rekey data */
+	struct {
+		u8 kck[NL80211_KCK_LEN], kek[NL80211_KEK_LEN];
+		__le64 replay_ctr;
+		bool valid;
+	} rekey_data;
 
 	struct list_head sta_list;
 };
-- 
2.29.2

[PATCH 09/13] wcn36xx: Add GTK offload to WoWLAN path

[Bryan O'Donoghue]

Using previously set GTK KCK and KEK material this commit adds GTK rekeying
to the WoWLAN suspend/resume path. A small error in the packing of the
up to now unused command structure is fixed as we go.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h  |  2 +-
 drivers/net/wireless/ath/wcn36xx/main.c |  4 +++
 drivers/net/wireless/ath/wcn36xx/smd.c  | 38 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  3 ++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 3b949b0b3792..1f3c2e840232 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -4905,7 +4905,7 @@ struct wcn36xx_hal_gtk_offload_req_msg {
 	u64 key_replay_counter;
 
 	u8 bss_index;
-};
+} __packed;
 
 struct wcn36xx_hal_gtk_offload_rsp_msg {
 	struct wcn36xx_hal_msg_header header;
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index b3388e4a76d2..aa2a416daa7f 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1121,6 +1121,9 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 		if (ret)
 			goto out;
 		ret = wcn36xx_smd_ipv6_ns_offload(wcn, vif, true);
+		if (ret)
+			goto out;
+		ret = wcn36xx_smd_gtk_offload(wcn, vif, true);
 		if (ret)
 			goto out;
 		ret = wcn36xx_smd_set_power_params(wcn, true);
@@ -1146,6 +1149,7 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 			goto out;
 
 		wcn36xx_smd_set_power_params(wcn, false);
+		wcn36xx_smd_gtk_offload(wcn, vif, false);
 		wcn36xx_smd_ipv6_ns_offload(wcn, vif, false);
 		wcn36xx_smd_arp_offload(wcn, vif, false);
 	}
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index b05f1e1f253a..db7988176526 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2849,6 +2849,43 @@ int wcn36xx_smd_ipv6_ns_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 	return ret;
 }
 
+int wcn36xx_smd_gtk_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
+			    bool enable)
+{
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	struct wcn36xx_hal_gtk_offload_req_msg msg_body;
+	int ret;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_GTK_OFFLOAD_REQ);
+
+	if (enable) {
+		memcpy(&msg_body.kek, vif_priv->rekey_data.kek, NL80211_KEK_LEN);
+		memcpy(&msg_body.kck, vif_priv->rekey_data.kck, NL80211_KCK_LEN);
+		msg_body.key_replay_counter = vif_priv->rekey_data.replay_ctr;
+		msg_body.bss_index = vif_priv->bss_index;
+	} else {
+		msg_body.flags = WCN36XX_HAL_GTK_OFFLOAD_FLAGS_DISABLE;
+	}
+
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
+	if (ret) {
+		wcn36xx_err("Sending host_offload_arp failed\n");
+		goto out;
+	}
+	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	if (ret) {
+		wcn36xx_err("host_offload_arp failed err=%d\n", ret);
+		goto out;
+	}
+out:
+	mutex_unlock(&wcn->hal_mutex);
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
@@ -2898,6 +2935,7 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 	case WCN36XX_HAL_START_SCAN_OFFLOAD_RSP:
 	case WCN36XX_HAL_STOP_SCAN_OFFLOAD_RSP:
 	case WCN36XX_HAL_HOST_OFFLOAD_RSP:
+	case WCN36XX_HAL_GTK_OFFLOAD_RSP:
 		memcpy(wcn->hal_buf, buf, len);
 		wcn->hal_rsp_len = len;
 		complete(&wcn->hal_rsp_compl);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 380ea58b58ec..dd6710672db8 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -153,4 +153,7 @@ int wcn36xx_smd_arp_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 int wcn36xx_smd_ipv6_ns_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 				bool enable);
 
+int wcn36xx_smd_gtk_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
+			    bool enable);
+
 #endif	/* _SMD_H_ */
-- 
2.29.2

[PATCH 10/13] wcn36xx: Add GTK offload info to WoWLAN resume

[Bryan O'Donoghue]

Having enabled GTK rekey in suspend, we need to extract the replay counter
from the firmware on resume and perform a ieee80211_gtk_rekey_notify() so
that the STA remains verified from the perspective of the AP.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h  |  4 +-
 drivers/net/wireless/ath/wcn36xx/main.c |  5 +-
 drivers/net/wireless/ath/wcn36xx/smd.c  | 72 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  3 ++
 4 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 1f3c2e840232..455143c4164e 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -4919,7 +4919,7 @@ struct wcn36xx_hal_gtk_offload_rsp_msg {
 struct wcn36xx_hal_gtk_offload_get_info_req_msg {
 	struct wcn36xx_hal_msg_header header;
 	u8 bss_index;
-};
+} __packed;
 
 struct wcn36xx_hal_gtk_offload_get_info_rsp_msg {
 	struct wcn36xx_hal_msg_header header;
@@ -4943,7 +4943,7 @@ struct wcn36xx_hal_gtk_offload_get_info_rsp_msg {
 	u32 igtk_rekey_count;
 
 	u8 bss_index;
-};
+} __packed;
 
 struct dhcp_info {
 	/* Indicates the device mode which indicates about the DHCP activity */
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index aa2a416daa7f..b48a3f0dcc0b 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -173,7 +173,9 @@ static struct ieee80211_supported_band wcn_band_5ghz = {
 #ifdef CONFIG_PM
 
 static const struct wiphy_wowlan_support wowlan_support = {
-	.flags = WIPHY_WOWLAN_ANY | WIPHY_WOWLAN_MAGIC_PKT
+	.flags = WIPHY_WOWLAN_ANY |
+		 WIPHY_WOWLAN_MAGIC_PKT |
+		 WIPHY_WOWLAN_SUPPORTS_GTK_REKEY
 };
 
 #endif
@@ -1149,6 +1151,7 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 			goto out;
 
 		wcn36xx_smd_set_power_params(wcn, false);
+		wcn36xx_smd_gtk_offload_get_info(wcn, vif);
 		wcn36xx_smd_gtk_offload(wcn, vif, false);
 		wcn36xx_smd_ipv6_ns_offload(wcn, vif, false);
 		wcn36xx_smd_arp_offload(wcn, vif, false);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index db7988176526..dd12575f33c3 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2886,6 +2886,77 @@ int wcn36xx_smd_gtk_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 	return ret;
 }
 
+static int wcn36xx_smd_gtk_offload_get_info_rsp(struct wcn36xx *wcn,
+						struct ieee80211_vif *vif)
+{
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	struct wcn36xx_hal_gtk_offload_get_info_rsp_msg *rsp;
+	__be64 replay_ctr;
+
+	if (wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len))
+		return -EIO;
+
+	rsp = (struct wcn36xx_hal_gtk_offload_get_info_rsp_msg *)wcn->hal_buf;
+
+	if (rsp->bss_index != vif_priv->bss_index) {
+		wcn36xx_err("gtk_offload_info invalid response bss index %d\n",
+			    rsp->bss_index);
+		return -ENOENT;
+	}
+
+	replay_ctr = cpu_to_be64(le64_to_cpu(rsp->key_replay_counter));
+	if (vif_priv->rekey_data.replay_ctr != rsp->key_replay_counter) {
+		vif_priv->rekey_data.replay_ctr = rsp->key_replay_counter;
+		ieee80211_gtk_rekey_notify(vif, vif->bss_conf.bssid,
+					   (void *)&replay_ctr, GFP_KERNEL);
+		 wcn36xx_dbg(WCN36XX_DBG_HAL,
+			     "GTK replay counter increment %llu\n",
+			     rsp->key_replay_counter);
+	}
+
+	wcn36xx_dbg(WCN36XX_DBG_HAL,
+		    "gtk offload info status %d last_rekey_status %d "
+		    "replay_counter %llu total_rekey_count %d gtk_rekey_count %d "
+		    "igtk_rekey_count %d bss_index %d\n",
+		    rsp->status, rsp->last_rekey_status,
+		    rsp->key_replay_counter, rsp->total_rekey_count,
+		    rsp->gtk_rekey_count, rsp->igtk_rekey_count,
+		    rsp->bss_index);
+
+	return 0;
+}
+
+int wcn36xx_smd_gtk_offload_get_info(struct wcn36xx *wcn,
+				     struct ieee80211_vif *vif)
+{
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	struct wcn36xx_hal_gtk_offload_get_info_req_msg msg_body;
+	int ret;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_GTK_OFFLOAD_GETINFO_REQ);
+
+	msg_body.bss_index = vif_priv->bss_index;
+
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
+	if (ret) {
+		wcn36xx_err("Sending gtk_offload_get_info failed\n");
+		goto out;
+	}
+	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	if (ret) {
+		wcn36xx_err("gtk_offload_get_info failed err=%d\n", ret);
+		goto out;
+	}
+	ret = wcn36xx_smd_gtk_offload_get_info_rsp(wcn, vif);
+out:
+	mutex_unlock(&wcn->hal_mutex);
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
@@ -2936,6 +3007,7 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 	case WCN36XX_HAL_STOP_SCAN_OFFLOAD_RSP:
 	case WCN36XX_HAL_HOST_OFFLOAD_RSP:
 	case WCN36XX_HAL_GTK_OFFLOAD_RSP:
+	case WCN36XX_HAL_GTK_OFFLOAD_GETINFO_RSP:
 		memcpy(wcn->hal_buf, buf, len);
 		wcn->hal_rsp_len = len;
 		complete(&wcn->hal_rsp_compl);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index dd6710672db8..b225c805107c 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -156,4 +156,7 @@ int wcn36xx_smd_ipv6_ns_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 int wcn36xx_smd_gtk_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 			    bool enable);
 
+int wcn36xx_smd_gtk_offload_get_info(struct wcn36xx *wcn,
+				     struct ieee80211_vif *vif);
+
 #endif	/* _SMD_H_ */
-- 
2.29.2

Re: [PATCH 10/13] wcn36xx: Add GTK offload info to WoWLAN resume

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Having enabled GTK rekey in suspend, we need to extract the replay counter
> from the firmware on resume and perform a ieee80211_gtk_rekey_notify() so
> that the STA remains verified from the perspective of the AP.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/hal.h  |  4 +-
>  drivers/net/wireless/ath/wcn36xx/main.c |  5 +-
>  drivers/net/wireless/ath/wcn36xx/smd.c  | 72 +++++++++++++++++++++++++
>  drivers/net/wireless/ath/wcn36xx/smd.h  |  3 ++
>  4 files changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
> index 1f3c2e840232..455143c4164e 100644
> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -4919,7 +4919,7 @@ struct wcn36xx_hal_gtk_offload_rsp_msg {
>  struct wcn36xx_hal_gtk_offload_get_info_req_msg {
>  	struct wcn36xx_hal_msg_header header;
>  	u8 bss_index;
> -};
> +} __packed;
>  
>  struct wcn36xx_hal_gtk_offload_get_info_rsp_msg {
>  	struct wcn36xx_hal_msg_header header;
> @@ -4943,7 +4943,7 @@ struct wcn36xx_hal_gtk_offload_get_info_rsp_msg {
>  	u32 igtk_rekey_count;
>  
>  	u8 bss_index;
> -};
> +} __packed;

Why __packed? Commit log mentions nothing about that.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 10/13] wcn36xx: Add GTK offload info to WoWLAN resume

[Bryan O'Donoghue]

On 11/01/2021 11:28, Kalle Valo wrote:
> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
> 
>> Having enabled GTK rekey in suspend, we need to extract the replay counter
>> from the firmware on resume and perform a ieee80211_gtk_rekey_notify() so
>> that the STA remains verified from the perspective of the AP.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   drivers/net/wireless/ath/wcn36xx/hal.h  |  4 +-
>>   drivers/net/wireless/ath/wcn36xx/main.c |  5 +-
>>   drivers/net/wireless/ath/wcn36xx/smd.c  | 72 +++++++++++++++++++++++++
>>   drivers/net/wireless/ath/wcn36xx/smd.h  |  3 ++
>>   4 files changed, 81 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
>> index 1f3c2e840232..455143c4164e 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
>> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
>> @@ -4919,7 +4919,7 @@ struct wcn36xx_hal_gtk_offload_rsp_msg {
>>   struct wcn36xx_hal_gtk_offload_get_info_req_msg {
>>   	struct wcn36xx_hal_msg_header header;
>>   	u8 bss_index;
>> -};
>> +} __packed;
>>   
>>   struct wcn36xx_hal_gtk_offload_get_info_rsp_msg {
>>   	struct wcn36xx_hal_msg_header header;
>> @@ -4943,7 +4943,7 @@ struct wcn36xx_hal_gtk_offload_get_info_rsp_msg {
>>   	u32 igtk_rekey_count;
>>   
>>   	u8 bss_index;
>> -};
>> +} __packed;
> 
> Why __packed? Commit log mentions nothing about that.
> 

Because the __u8 at the end aligns to 4 bytes.

I'll add this to the log desc

Re: [PATCH 10/13] wcn36xx: Add GTK offload info to WoWLAN resume

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Having enabled GTK rekey in suspend, we need to extract the replay counter
> from the firmware on resume and perform a ieee80211_gtk_rekey_notify() so
> that the STA remains verified from the perspective of the AP.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[...]

>  static const struct wiphy_wowlan_support wowlan_support = {
> -	.flags = WIPHY_WOWLAN_ANY | WIPHY_WOWLAN_MAGIC_PKT
> +	.flags = WIPHY_WOWLAN_ANY |
> +		 WIPHY_WOWLAN_MAGIC_PKT |
> +		 WIPHY_WOWLAN_SUPPORTS_GTK_REKEY
>  };

I wonder should these also be in the last patch? So that the features
are enabled only after everything is implemented.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

[PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Bryan O'Donoghue]

If a scan is in progress do not attempt to enter into suspend. Allow the
scan process to quiesce before proceeding.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c    |  5 +++++
 drivers/net/wireless/ath/wcn36xx/smd.c     | 13 +++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h     |  2 ++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |  1 +
 4 files changed, 21 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index b48a3f0dcc0b..feb909192c8e 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1113,6 +1113,11 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac suspend\n");
 
+	if (wcn36xx_smd_is_scanning(wcn)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
 	mutex_lock(&wcn->conf_mutex);
 	vif = wcn36xx_get_first_vif(wcn);
 	if (vif) {
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index dd12575f33c3..378282a93aa0 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -731,6 +731,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
 		wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
 		goto out;
 	}
+	wcn->scanning = true;
 out:
 	mutex_unlock(&wcn->hal_mutex);
 	return ret;
@@ -807,6 +808,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
 	mutex_lock(&wcn->hal_mutex);
 	INIT_HAL_MSG(msg_body, WCN36XX_HAL_FINISH_SCAN_REQ);
 
+	wcn->scanning = false;
 	msg_body.mode = mode;
 	msg_body.oper_channel = WCN36XX_HW_CHANNEL(wcn);
 	if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX) {
@@ -938,6 +940,17 @@ int wcn36xx_smd_stop_hw_scan(struct wcn36xx *wcn)
 	return ret;
 }
 
+bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn)
+{
+	bool scanning;
+
+	mutex_lock(&wcn->hal_mutex);
+	scanning = wcn->scanning;
+	mutex_unlock(&wcn->hal_mutex);
+
+	return scanning;
+}
+
 static int wcn36xx_smd_switch_channel_rsp(void *buf, size_t len)
 {
 	struct wcn36xx_hal_switch_channel_rsp_msg *rsp;
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index b225c805107c..3488abb201d0 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -159,4 +159,6 @@ int wcn36xx_smd_gtk_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
 int wcn36xx_smd_gtk_offload_get_info(struct wcn36xx *wcn,
 				     struct ieee80211_vif *vif);
 
+bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn);
+
 #endif	/* _SMD_H_ */
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 6121d8a5641a..36ea768a5203 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -249,6 +249,7 @@ struct wcn36xx {
 	struct ieee80211_vif	*sw_scan_vif;
 	struct mutex		scan_lock;
 	bool			scan_aborted;
+	bool			scanning;
 
 	/* DXE channels */
 	struct wcn36xx_dxe_ch	dxe_tx_l_ch;	/* TX low */
-- 
2.29.2

Re: [PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Benjamin Li]

On 12/28/20 8:28 AM, Bryan O'Donoghue wrote:
> If a scan is in progress do not attempt to enter into suspend. Allow the
> scan process to quiesce before proceeding.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/main.c    |  5 +++++
>   drivers/net/wireless/ath/wcn36xx/smd.c     | 13 +++++++++++++
>   drivers/net/wireless/ath/wcn36xx/smd.h     |  2 ++
>   drivers/net/wireless/ath/wcn36xx/wcn36xx.h |  1 +
>   4 files changed, 21 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index b48a3f0dcc0b..feb909192c8e 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -1113,6 +1113,11 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
>   
>   	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac suspend\n");
>   
> +	if (wcn36xx_smd_is_scanning(wcn)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +

Should just be a return, since we haven't locked conf_mutex yet?

>   	mutex_lock(&wcn->conf_mutex);
>   	vif = wcn36xx_get_first_vif(wcn);
>   	if (vif) {
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index dd12575f33c3..378282a93aa0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -731,6 +731,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
>   		wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
>   		goto out;
>   	}
> +	wcn->scanning = true;
>   out:
>   	mutex_unlock(&wcn->hal_mutex);
>   	return ret;
> @@ -807,6 +808,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
>   	mutex_lock(&wcn->hal_mutex);
>   	INIT_HAL_MSG(msg_body, WCN36XX_HAL_FINISH_SCAN_REQ);
>   
> +	wcn->scanning = false;
>   	msg_body.mode = mode;
>   	msg_body.oper_channel = WCN36XX_HW_CHANNEL(wcn);
>   	if (vif_priv->bss_index != WCN36XX_HAL_BSS_INVALID_IDX) {
> @@ -938,6 +940,17 @@ int wcn36xx_smd_stop_hw_scan(struct wcn36xx *wcn)
>   	return ret;
>   }
>   
> +bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn)
> +{
> +	bool scanning;
> +
> +	mutex_lock(&wcn->hal_mutex);
> +	scanning = wcn->scanning;
> +	mutex_unlock(&wcn->hal_mutex);
> +
> +	return scanning;
> +}
> +
>   static int wcn36xx_smd_switch_channel_rsp(void *buf, size_t len)
>   {
>   	struct wcn36xx_hal_switch_channel_rsp_msg *rsp;
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index b225c805107c..3488abb201d0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -159,4 +159,6 @@ int wcn36xx_smd_gtk_offload(struct wcn36xx *wcn, struct ieee80211_vif *vif,
>   int wcn36xx_smd_gtk_offload_get_info(struct wcn36xx *wcn,
>   				     struct ieee80211_vif *vif);
>   
> +bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn);
> +
>   #endif	/* _SMD_H_ */
> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> index 6121d8a5641a..36ea768a5203 100644
> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> @@ -249,6 +249,7 @@ struct wcn36xx {
>   	struct ieee80211_vif	*sw_scan_vif;
>   	struct mutex		scan_lock;
>   	bool			scan_aborted;
> +	bool			scanning;
>   
>   	/* DXE channels */
>   	struct wcn36xx_dxe_ch	dxe_tx_l_ch;	/* TX low */
> 

Re: [PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> If a scan is in progress do not attempt to enter into suspend. Allow the
> scan process to quiesce before proceeding.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Why? I would have considered the opposite and if we go to suspend we
cancel the scan. No strong feelings, just don't see the need for scan
results during suspend. But of course I might be missing something...

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Bryan O'Donoghue]

On 11/01/2021 11:31, Kalle Valo wrote:
> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
> 
>> If a scan is in progress do not attempt to enter into suspend. Allow the
>> scan process to quiesce before proceeding.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 
> Why? I would have considered the opposite and if we go to suspend we
> cancel the scan. No strong feelings, just don't see the need for scan
> results during suspend. But of course I might be missing something...

We need to be switched to the AP's channel when calling the suspend 
routine. During a s/w scan we switch off channel to scan for 100s of 
milliseconds.

If the suspend() routine is called while that is true, we suspend on the 
wrong channel.

So we would need to switch to the right channel explicitly in suspend 
but, at the moment wcn36xx_config() for switching channels and I thought 
it best to leave the channel switching logic in the one place.

I'm not opposed in principle to

- Entering suspend
- Switching to the last known active channel
- Suspending

---
bod

Re: [PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> On 11/01/2021 11:31, Kalle Valo wrote:
>> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
>>
>>> If a scan is in progress do not attempt to enter into suspend. Allow the
>>> scan process to quiesce before proceeding.
>>>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>
>> Why? I would have considered the opposite and if we go to suspend we
>> cancel the scan. No strong feelings, just don't see the need for scan
>> results during suspend. But of course I might be missing something...
>
> We need to be switched to the AP's channel when calling the suspend
> routine. During a s/w scan we switch off channel to scan for 100s of
> milliseconds.
>
> If the suspend() routine is called while that is true, we suspend on
> the wrong channel.
>
> So we would need to switch to the right channel explicitly in suspend
> but, at the moment wcn36xx_config() for switching channels and I
> thought it best to leave the channel switching logic in the one place.
>
> I'm not opposed in principle to
>
> - Entering suspend
> - Switching to the last known active channel
> - Suspending

Should this be fixed in mac80211? Otherwise every driver using software
scan needs to have a workaround for this, right?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Bryan O'Donoghue]

On 11/01/2021 12:26, Kalle Valo wrote:
> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
> 
>> On 11/01/2021 11:31, Kalle Valo wrote:
>>> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:
>>>
>>>> If a scan is in progress do not attempt to enter into suspend. Allow the
>>>> scan process to quiesce before proceeding.
>>>>
>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>
>>> Why? I would have considered the opposite and if we go to suspend we
>>> cancel the scan. No strong feelings, just don't see the need for scan
>>> results during suspend. But of course I might be missing something...
>>
>> We need to be switched to the AP's channel when calling the suspend
>> routine. During a s/w scan we switch off channel to scan for 100s of
>> milliseconds.
>>
>> If the suspend() routine is called while that is true, we suspend on
>> the wrong channel.
>>
>> So we would need to switch to the right channel explicitly in suspend
>> but, at the moment wcn36xx_config() for switching channels and I
>> thought it best to leave the channel switching logic in the one place.
>>
>> I'm not opposed in principle to
>>
>> - Entering suspend
>> - Switching to the last known active channel
>> - Suspending
> 
> Should this be fixed in mac80211? Otherwise every driver using software
> scan needs to have a workaround for this, right?
> 

I'll check.

I thought this problem was likely specific to wcn36xx but, conceptually 
I can't argue with you there.

Given I only see this behavior on Android and not on Debian - I test 
both - where Android tends to scan constantly - it is possible I'm the 
only one really triggering the bug given most drivers don't do s/w scan 
and probably wcn36xx is the only s/w scan being used on an Android 
system close to what we have upstream.

So yeah fair point, I'll see if the fix fits better at a higher level.

---
bod

Re: [PATCH 11/13] wcn36xx: Do not suspend if scan in progress

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> If a scan is in progress do not attempt to enter into suspend. Allow the
> scan process to quiesce before proceeding.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[...]

> +bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn)
> +{
> +	bool scanning;
> +
> +	mutex_lock(&wcn->hal_mutex);
> +	scanning = wcn->scanning;
> +	mutex_unlock(&wcn->hal_mutex);
> +
> +	return scanning;
> +}

Instead of having a bool you could use SET_BIT() & co, that way you
don't need this extra function. For example see dev_flags in ath10k.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

[PATCH 12/13] wcn36xx: Add Host suspend indication support

[Bryan O'Donoghue]

In order to activate ipv4 ARP offload, ipv6 NS offload and firmware GTK
offload we need to send a unidirectional indication from host to wcn
indicating a transition to suspend.

Once done, firmware will respond to ARP broadcasts, ipv6 NS lookups and
perform GTK rekeys without waking the host.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c |  3 +++
 drivers/net/wireless/ath/wcn36xx/smd.c  | 19 +++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  2 ++
 3 files changed, 24 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index feb909192c8e..ec24b0cc5b8c 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1134,6 +1134,9 @@ static int wcn36xx_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wow)
 		if (ret)
 			goto out;
 		ret = wcn36xx_smd_set_power_params(wcn, true);
+		if (ret)
+			goto out;
+		ret = wcn36xx_smd_wlan_host_suspend_ind(wcn);
 	}
 out:
 	mutex_unlock(&wcn->conf_mutex);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 378282a93aa0..5450d56d8e1c 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2970,6 +2970,25 @@ int wcn36xx_smd_gtk_offload_get_info(struct wcn36xx *wcn,
 	return ret;
 }
 
+int wcn36xx_smd_wlan_host_suspend_ind(struct wcn36xx *wcn)
+{
+	struct wcn36xx_hal_wlan_host_suspend_ind_msg msg_body;
+	int ret;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_HOST_SUSPEND_IND);
+	msg_body.configured_mcst_bcst_filter_setting = 0;
+	msg_body.active_session_count = 1;
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	ret = rpmsg_send(wcn->smd_channel, wcn->hal_buf, msg_body.header.len);
+
+	mutex_unlock(&wcn->hal_mutex);
+
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 3488abb201d0..77efcfa145ec 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -161,4 +161,6 @@ int wcn36xx_smd_gtk_offload_get_info(struct wcn36xx *wcn,
 
 bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn);
 
+int wcn36xx_smd_wlan_host_suspend_ind(struct wcn36xx *wcn);
+
 #endif	/* _SMD_H_ */
-- 
2.29.2

[PATCH 13/13] wcn36xx: Add host resume request support

[Bryan O'Donoghue]

This commit is the corresponding resume() path request to the firmware when
resuming. Unlike the suspend() version which is a unidirectional
indication, the resume version is a standard request/response.

Once the resume() request completes ipv4 ARP, ipv6 NS and GTK rekey offload
stop working and can subsequently be rolled back.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c |  1 +
 drivers/net/wireless/ath/wcn36xx/smd.c  | 35 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  2 ++
 3 files changed, 38 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index ec24b0cc5b8c..9a2a6399c8a0 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1158,6 +1158,7 @@ static int wcn36xx_resume(struct ieee80211_hw *hw)
 		if (!vif_priv->sta_assoc)
 			goto out;
 
+		wcn36xx_smd_host_resume(wcn);
 		wcn36xx_smd_set_power_params(wcn, false);
 		wcn36xx_smd_gtk_offload_get_info(wcn, vif);
 		wcn36xx_smd_gtk_offload(wcn, vif, false);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 5450d56d8e1c..83452d7f4634 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2989,6 +2989,40 @@ int wcn36xx_smd_wlan_host_suspend_ind(struct wcn36xx *wcn)
 	return ret;
 }
 
+int wcn36xx_smd_host_resume(struct wcn36xx *wcn)
+{
+	struct wcn36xx_hal_wlan_host_resume_req_msg msg_body;
+	struct wcn36xx_hal_host_resume_rsp_msg *rsp;
+	int ret;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_HOST_RESUME_REQ);
+	msg_body.configured_mcst_bcst_filter_setting = 0;
+
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
+	if (ret) {
+		wcn36xx_err("Sending wlan_host_resume failed\n");
+		goto out;
+	}
+	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	if (ret) {
+		wcn36xx_err("wlan_host_resume err=%d\n", ret);
+		goto out;
+	}
+
+	rsp = (struct wcn36xx_hal_host_resume_rsp_msg *)wcn->hal_buf;
+	if (rsp->status)
+		wcn36xx_warn("wlan_host_resume status=%d\n", rsp->status);
+
+out:
+	mutex_unlock(&wcn->hal_mutex);
+
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
@@ -3040,6 +3074,7 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 	case WCN36XX_HAL_HOST_OFFLOAD_RSP:
 	case WCN36XX_HAL_GTK_OFFLOAD_RSP:
 	case WCN36XX_HAL_GTK_OFFLOAD_GETINFO_RSP:
+	case WCN36XX_HAL_HOST_RESUME_RSP:
 		memcpy(wcn->hal_buf, buf, len);
 		wcn->hal_rsp_len = len;
 		complete(&wcn->hal_rsp_compl);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 77efcfa145ec..6dfed8f386d9 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -163,4 +163,6 @@ bool wcn36xx_smd_is_scanning(struct wcn36xx *wcn);
 
 int wcn36xx_smd_wlan_host_suspend_ind(struct wcn36xx *wcn);
 
+int wcn36xx_smd_host_resume(struct wcn36xx *wcn);
+
 #endif	/* _SMD_H_ */
-- 
2.29.2