diff mbox

[3/3] ath10k: average ack rssi support for data frames

Message ID 1523619357-1963-4-git-send-email-bpothuno@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Balaji Pothunoori April 13, 2018, 11:35 a.m. UTC
Average ack rssi value is weighted average of ack rssi for
no of msdu's has been sent.
This feature is enabled by the host driver if firmware is capable.
After receiving event from host, firmware allocates the necessary
memory to store the ack_rssi for data packets during the init time.

After each successful transmission, If tx completion status is OK
and 24th bit is set in HTT message header then host will fetch the
ack_rssi else host can ignore the ack_rssi field.

Note: This patch is depends on cfg80211 patch.

Signed-off-by: Balaji Pothunoori <bpothuno@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c   |  4 ++++
 drivers/net/wireless/ath/ath10k/htt.h    |  6 +++++-
 drivers/net/wireless/ath/ath10k/htt_rx.c | 21 +++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/mac.c    |  4 ++++
 drivers/net/wireless/ath/ath10k/wmi.h    |  8 ++++++++
 5 files changed, 42 insertions(+), 1 deletion(-)

Comments

Kalle Valo April 13, 2018, 1:56 p.m. UTC | #1
Balaji Pothunoori <bpothuno@codeaurora.org> writes:

> Average ack rssi value is weighted average of ack rssi for
> no of msdu's has been sent.
> This feature is enabled by the host driver if firmware is capable.
> After receiving event from host, firmware allocates the necessary
> memory to store the ack_rssi for data packets during the init time.
>
> After each successful transmission, If tx completion status is OK
> and 24th bit is set in HTT message header then host will fetch the
> ack_rssi else host can ignore the ack_rssi field.
>
> Note: This patch is depends on cfg80211 patch.
>
> Signed-off-by: Balaji Pothunoori <bpothuno@codeaurora.org>
> ---
>  drivers/net/wireless/ath/ath10k/core.c   |  4 ++++
>  drivers/net/wireless/ath/ath10k/htt.h    |  6 +++++-
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 21 +++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/mac.c    |  4 ++++
>  drivers/net/wireless/ath/ath10k/wmi.h    |  8 ++++++++
>  5 files changed, 42 insertions(+), 1 deletion(-)

Usually notes like above are better to put under "---" line, this way
git-am will not include in the git commit log.
kernel test robot April 14, 2018, 6:44 a.m. UTC | #2
Hi Balaji,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180411]
[cannot apply to ath6kl/ath-next v4.16 v4.16-rc7 v4.16-rc6 v4.16]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Balaji-Pothunoori/cfg80211-average-ack-rssi-support-for-data-frames/20180414-115825
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_t2h_msg_handler':
>> drivers/net/wireless/ath/ath10k/htt_rx.c:1886:9: warning: 'msdu_count' may be used uninitialized in this function [-Wmaybe-uninitialized]
     __le16 msdu_count;
            ^~~~~~~~~~

vim +/msdu_count +1886 drivers/net/wireless/ath/ath10k/htt_rx.c

  1875	
  1876	static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
  1877					       struct sk_buff *skb)
  1878	{
  1879		struct ath10k_htt *htt = &ar->htt;
  1880		struct htt_resp *resp = (struct htt_resp *)skb->data;
  1881		struct htt_tx_done tx_done = {};
  1882		int status = MS(resp->data_tx_completion.flags, HTT_DATA_TX_STATUS);
  1883		__le16 msdu_id;
  1884		int i;
  1885		bool rssi_enabled;
> 1886		__le16 msdu_count;
  1887	
  1888		switch (status) {
  1889		case HTT_DATA_TX_STATUS_NO_ACK:
  1890			tx_done.status = HTT_TX_COMPL_STATE_NOACK;
  1891			break;
  1892		case HTT_DATA_TX_STATUS_OK:
  1893			tx_done.status = HTT_TX_COMPL_STATE_ACK;
  1894			break;
  1895		case HTT_DATA_TX_STATUS_DISCARD:
  1896		case HTT_DATA_TX_STATUS_POSTPONE:
  1897		case HTT_DATA_TX_STATUS_DOWNLOAD_FAIL:
  1898			tx_done.status = HTT_TX_COMPL_STATE_DISCARD;
  1899			break;
  1900		default:
  1901			ath10k_warn(ar, "unhandled tx completion status %d\n", status);
  1902			tx_done.status = HTT_TX_COMPL_STATE_DISCARD;
  1903			break;
  1904		}
  1905	
  1906		ath10k_dbg(ar, ATH10K_DBG_HTT, "htt tx completion num_msdus %d\n",
  1907			   resp->data_tx_completion.num_msdus);
  1908	
  1909		if (resp->data_tx_completion.flags2 & HTT_TX_CMPL_FLAG_DATA_RSSI) {
  1910			rssi_enabled = true;
  1911			msdu_count = __le16_to_cpu(resp->data_tx_completion.num_msdus);
  1912		} else {
  1913			rssi_enabled = false;
  1914		}
  1915	
  1916		for (i = 0; i < resp->data_tx_completion.num_msdus; i++) {
  1917			msdu_id = resp->data_tx_completion.msdus[i];
  1918			tx_done.msdu_id = __le16_to_cpu(msdu_id);
  1919	
  1920			if (rssi_enabled) {
  1921				/* Total no of MSDUs should be even,
  1922				 * if odd MSDUs are sent firmware fills
  1923				 * last msdu id with 0xffff
  1924				 */
  1925				if (msdu_count & 0x01)
  1926					tx_done.ack_rssi =
  1927					resp->data_tx_completion.msdus[msdu_count +  i + 1];
  1928				else
  1929					tx_done.ack_rssi =
  1930					resp->data_tx_completion.msdus[msdu_count + i];
  1931			}
  1932			/* kfifo_put: In practice firmware shouldn't fire off per-CE
  1933			 * interrupt and main interrupt (MSI/-X range case) for the same
  1934			 * HTC service so it should be safe to use kfifo_put w/o lock.
  1935			 *
  1936			 * From kfifo_put() documentation:
  1937			 *  Note that with only one concurrent reader and one concurrent
  1938			 *  writer, you don't need extra locking to use these macro.
  1939			 */
  1940			if (!kfifo_put(&htt->txdone_fifo, tx_done)) {
  1941				ath10k_warn(ar, "txdone fifo overrun, msdu_id %d status %d\n",
  1942					    tx_done.msdu_id, tx_done.status);
  1943				ath10k_txrx_tx_unref(htt, &tx_done);
  1944			}
  1945		}
  1946	}
  1947	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot April 14, 2018, 6:56 a.m. UTC | #3
Hi Balaji,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180411]
[cannot apply to ath6kl/ath-next v4.16 v4.16-rc7 v4.16-rc6 v4.16]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Balaji-Pothunoori/cfg80211-average-ack-rssi-support-for-data-frames/20180414-115825
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/net/wireless/ath/ath10k/htt_rx.c:236:23: sparse: expression using sizeof(void)
   drivers/net/wireless/ath/ath10k/htt_rx.c:389:31: sparse: expression using sizeof(void)
   drivers/net/wireless/ath/ath10k/htt_rx.c:402:39: sparse: expression using sizeof(void)
>> drivers/net/wireless/ath/ath10k/htt_rx.c:1911:30: sparse: cast to restricted __le16
>> drivers/net/wireless/ath/ath10k/htt_rx.c:1911:28: sparse: incorrect type in assignment (different base types) @@    expected restricted __le16 [usertype] msdu_count @@    got unsignedrestricted __le16 [usertype] msdu_count @@
   drivers/net/wireless/ath/ath10k/htt_rx.c:1911:28:    expected restricted __le16 [usertype] msdu_count
   drivers/net/wireless/ath/ath10k/htt_rx.c:1911:28:    got unsigned short [unsigned] [usertype] <noident>
>> drivers/net/wireless/ath/ath10k/htt_rx.c:1925:29: sparse: restricted __le16 degrades to integer
   drivers/net/wireless/ath/ath10k/htt_rx.c:1927:64: sparse: restricted __le16 degrades to integer
>> drivers/net/wireless/ath/ath10k/htt_rx.c:1926:50: sparse: incorrect type in assignment (different base types) @@    expected unsigned char [unsigned] [assigned] [usertype] ack_rssi @@    got igned] [usertype] ack_rssi @@
   drivers/net/wireless/ath/ath10k/htt_rx.c:1926:50:    expected unsigned char [unsigned] [assigned] [usertype] ack_rssi
   drivers/net/wireless/ath/ath10k/htt_rx.c:1926:50:    got restricted __le16 <noident>
   drivers/net/wireless/ath/ath10k/htt_rx.c:1930:64: sparse: restricted __le16 degrades to integer
   drivers/net/wireless/ath/ath10k/htt_rx.c:1929:50: sparse: incorrect type in assignment (different base types) @@    expected unsigned char [unsigned] [assigned] [usertype] ack_rssi @@    got igned] [usertype] ack_rssi @@
   drivers/net/wireless/ath/ath10k/htt_rx.c:1929:50:    expected unsigned char [unsigned] [assigned] [usertype] ack_rssi
   drivers/net/wireless/ath/ath10k/htt_rx.c:1929:50:    got restricted __le16 <noident>
   drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_t2h_msg_handler':
   drivers/net/wireless/ath/ath10k/htt_rx.c:1886:9: warning: 'msdu_count' may be used uninitialized in this function [-Wmaybe-uninitialized]
     __le16 msdu_count;
            ^~~~~~~~~~

vim +1911 drivers/net/wireless/ath/ath10k/htt_rx.c

  1875	
  1876	static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
  1877					       struct sk_buff *skb)
  1878	{
  1879		struct ath10k_htt *htt = &ar->htt;
  1880		struct htt_resp *resp = (struct htt_resp *)skb->data;
  1881		struct htt_tx_done tx_done = {};
  1882		int status = MS(resp->data_tx_completion.flags, HTT_DATA_TX_STATUS);
  1883		__le16 msdu_id;
  1884		int i;
  1885		bool rssi_enabled;
  1886		__le16 msdu_count;
  1887	
  1888		switch (status) {
  1889		case HTT_DATA_TX_STATUS_NO_ACK:
  1890			tx_done.status = HTT_TX_COMPL_STATE_NOACK;
  1891			break;
  1892		case HTT_DATA_TX_STATUS_OK:
  1893			tx_done.status = HTT_TX_COMPL_STATE_ACK;
  1894			break;
  1895		case HTT_DATA_TX_STATUS_DISCARD:
  1896		case HTT_DATA_TX_STATUS_POSTPONE:
  1897		case HTT_DATA_TX_STATUS_DOWNLOAD_FAIL:
  1898			tx_done.status = HTT_TX_COMPL_STATE_DISCARD;
  1899			break;
  1900		default:
  1901			ath10k_warn(ar, "unhandled tx completion status %d\n", status);
  1902			tx_done.status = HTT_TX_COMPL_STATE_DISCARD;
  1903			break;
  1904		}
  1905	
  1906		ath10k_dbg(ar, ATH10K_DBG_HTT, "htt tx completion num_msdus %d\n",
  1907			   resp->data_tx_completion.num_msdus);
  1908	
  1909		if (resp->data_tx_completion.flags2 & HTT_TX_CMPL_FLAG_DATA_RSSI) {
  1910			rssi_enabled = true;
> 1911			msdu_count = __le16_to_cpu(resp->data_tx_completion.num_msdus);
  1912		} else {
  1913			rssi_enabled = false;
  1914		}
  1915	
  1916		for (i = 0; i < resp->data_tx_completion.num_msdus; i++) {
  1917			msdu_id = resp->data_tx_completion.msdus[i];
  1918			tx_done.msdu_id = __le16_to_cpu(msdu_id);
  1919	
  1920			if (rssi_enabled) {
  1921				/* Total no of MSDUs should be even,
  1922				 * if odd MSDUs are sent firmware fills
  1923				 * last msdu id with 0xffff
  1924				 */
> 1925				if (msdu_count & 0x01)
> 1926					tx_done.ack_rssi =
  1927					resp->data_tx_completion.msdus[msdu_count +  i + 1];
  1928				else
  1929					tx_done.ack_rssi =
  1930					resp->data_tx_completion.msdus[msdu_count + i];
  1931			}
  1932			/* kfifo_put: In practice firmware shouldn't fire off per-CE
  1933			 * interrupt and main interrupt (MSI/-X range case) for the same
  1934			 * HTC service so it should be safe to use kfifo_put w/o lock.
  1935			 *
  1936			 * From kfifo_put() documentation:
  1937			 *  Note that with only one concurrent reader and one concurrent
  1938			 *  writer, you don't need extra locking to use these macro.
  1939			 */
  1940			if (!kfifo_put(&htt->txdone_fifo, tx_done)) {
  1941				ath10k_warn(ar, "txdone fifo overrun, msdu_id %d status %d\n",
  1942					    tx_done.msdu_id, tx_done.status);
  1943				ath10k_txrx_tx_unref(htt, &tx_done);
  1944			}
  1945		}
  1946	}
  1947	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 8a3020d..0fe98e9 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2307,6 +2307,10 @@  int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 			     ar->wmi.svc_map))
 			val |= WMI_10_4_TDLS_UAPSD_BUFFER_STA;
 
+		if (test_bit(WMI_SERVICE_TX_DATA_ACK_RSSI,
+			     ar->wmi.svc_map))
+			val |= WMI_10_4_TX_DATA_ACK_RSSI;
+
 		status = ath10k_mac_ext_resource_config(ar, val);
 		if (status) {
 			ath10k_err(ar,
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 8cc2a8b..bd39d6b 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1,6 +1,7 @@ 
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -557,6 +558,8 @@  struct htt_mgmt_tx_completion {
 #define HTT_RX_INDICATION_INFO1_NUM_MPDU_RANGES_MASK     0xFF000000
 #define HTT_RX_INDICATION_INFO1_NUM_MPDU_RANGES_LSB      24
 
+#define HTT_TX_CMPL_FLAG_DATA_RSSI BIT(0)
+
 struct htt_rx_indication_hdr {
 	u8 info0; /* %HTT_RX_INDICATION_INFO0_ */
 	__le16 peer_id;
@@ -820,7 +823,7 @@  struct htt_data_tx_completion {
 		} __packed;
 	} __packed;
 	u8 num_msdus;
-	u8 rsvd0;
+	u8 flags2; /* HTT_TX_CMPL_FLAG_DATA_RSSI */
 	__le16 msdus[0]; /* variable length based on %num_msdus */
 } __packed;
 
@@ -1648,6 +1651,7 @@  struct htt_resp {
 struct htt_tx_done {
 	u16 msdu_id;
 	u16 status;
+	u8 ack_rssi;
 };
 
 enum htt_tx_compl_state {
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 5e02e26..e16e818 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1882,6 +1882,8 @@  static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
 	int status = MS(resp->data_tx_completion.flags, HTT_DATA_TX_STATUS);
 	__le16 msdu_id;
 	int i;
+	bool rssi_enabled;
+	__le16 msdu_count;
 
 	switch (status) {
 	case HTT_DATA_TX_STATUS_NO_ACK:
@@ -1904,10 +1906,29 @@  static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
 	ath10k_dbg(ar, ATH10K_DBG_HTT, "htt tx completion num_msdus %d\n",
 		   resp->data_tx_completion.num_msdus);
 
+	if (resp->data_tx_completion.flags2 & HTT_TX_CMPL_FLAG_DATA_RSSI) {
+		rssi_enabled = true;
+		msdu_count = __le16_to_cpu(resp->data_tx_completion.num_msdus);
+	} else {
+		rssi_enabled = false;
+	}
+
 	for (i = 0; i < resp->data_tx_completion.num_msdus; i++) {
 		msdu_id = resp->data_tx_completion.msdus[i];
 		tx_done.msdu_id = __le16_to_cpu(msdu_id);
 
+		if (rssi_enabled) {
+			/* Total no of MSDUs should be even,
+			 * if odd MSDUs are sent firmware fills
+			 * last msdu id with 0xffff
+			 */
+			if (msdu_count & 0x01)
+				tx_done.ack_rssi =
+				resp->data_tx_completion.msdus[msdu_count +  i + 1];
+			else
+				tx_done.ack_rssi =
+				resp->data_tx_completion.msdus[msdu_count + i];
+		}
 		/* kfifo_put: In practice firmware shouldn't fire off per-CE
 		 * interrupt and main interrupt (MSI/-X range case) for the same
 		 * HTC service so it should be safe to use kfifo_put w/o lock.
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bf05a36..f62ddd6 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8352,6 +8352,10 @@  int ath10k_mac_register(struct ath10k *ar)
 
 	wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_VHT_IBSS);
 
+	if (test_bit(WMI_SERVICE_TX_DATA_ACK_RSSI, ar->wmi.svc_map))
+		wiphy_ext_feature_set(ar->hw->wiphy,
+				NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT);
+
 	/*
 	 * on LL hardware queues are managed entirely by the FW
 	 * so we only advertise to mac we can do the queues thing
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 6fbc84c..5c43c26 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -201,6 +201,7 @@  enum wmi_service {
 	WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
 	WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
 	WMI_SERVICE_TPC_STATS_FINAL,
+	WMI_SERVICE_TX_DATA_ACK_RSSI,
 
 	/* keep last */
 	WMI_SERVICE_MAX,
@@ -346,6 +347,8 @@  enum wmi_10_4_service {
 	WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
 	WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
 	WMI_10_4_SERVICE_TPC_STATS_FINAL,
+	WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+	WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
 };
 
 static inline char *wmi_service_name(int service_id)
@@ -458,6 +461,7 @@  static inline char *wmi_service_name(int service_id)
 	SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
 	SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
 	SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
+	SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
 	default:
 		return NULL;
 	}
@@ -762,6 +766,8 @@  static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
 	       WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
 	SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
 	       WMI_SERVICE_TPC_STATS_FINAL, len);
+	SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+	       WMI_SERVICE_TX_DATA_ACK_RSSI, len);
 }
 
 #undef SVCMAP
@@ -2904,6 +2910,7 @@  enum wmi_coex_version {
  * @WMI_10_4_TDLS_CONN_TRACKER_IN_HOST_MODE: TDLS connection tracker in host
  *	enable/disable
  * @WMI_10_4_TDLS_EXPLICIT_MODE_ONLY:Explicit TDLS mode enable/disable
+ * @WMI_10_4_TX_DATA_ACK_RSSI: Enable DATA ACK RSSI if firmware is capable
  */
 enum wmi_10_4_feature_mask {
 	WMI_10_4_LTEU_SUPPORT			= BIT(0),
@@ -2919,6 +2926,7 @@  enum wmi_10_4_feature_mask {
 	WMI_10_4_TDLS_UAPSD_SLEEP_STA		= BIT(10),
 	WMI_10_4_TDLS_CONN_TRACKER_IN_HOST_MODE = BIT(11),
 	WMI_10_4_TDLS_EXPLICIT_MODE_ONLY	= BIT(12),
+	WMI_10_4_TX_DATA_ACK_RSSI               = BIT(16),
 
 };