[PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Wen Gong]

For sdio chip, it is high latency bus, all the TX packet's content will
be tranferred from HOST memory to firmware memory via sdio bus, then it
need much more memory in firmware than low latency bus chip, for low
latency chip, such as PCI-E, it only need to transfer the TX descriptor
via PCI-E bus to firmware memory. For sdio chip, reduce the complexity of
TX logic will help TX efficiency since its memory is limited, and it will
reduce the TX circle's time of each packet and then firmware will have more
memory for TX since TX complete also need memeory.

This patch disable TX complete indication from firmware for htt data
packet, it will not have TX complete indication from firmware to ath10k.
It will cut the cost of bus bandwidth of TX complete and make the TX
logic of firmware simpler, it results in significant performance
improvement on TX path.

Udp TX throughout is 130Mbps without this patch, and it arrives
400Mbps with this patch.

The downside of this patch is the command "iw wlan0 station dump" will
show 0 for "tx retries" and "tx failed" since all tx packet's status
is success.

This patch only effect sdio chip, it will not effect PCI, SNOC etc.

Tested with QCA6174 SDIO with firmware
WLAN.RMH.4.4.1-00017-QCARMSWPZ-1

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c   |  5 +---
 drivers/net/wireless/ath/ath10k/hif.h    |  9 +++++++
 drivers/net/wireless/ath/ath10k/htc.c    | 10 +++++++
 drivers/net/wireless/ath/ath10k/htc.h    |  3 +++
 drivers/net/wireless/ath/ath10k/htt.c    |  5 ++++
 drivers/net/wireless/ath/ath10k/htt.h    | 13 ++++++++-
 drivers/net/wireless/ath/ath10k/htt_rx.c | 34 +++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/htt_tx.c | 31 +++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h     |  2 +-
 drivers/net/wireless/ath/ath10k/sdio.c   | 23 ++++++++++++++++
 10 files changed, 128 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 4f76ba5d78a9..5a49edfdf9e6 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -695,10 +695,7 @@ static int ath10k_init_sdio(struct ath10k *ar, enum ath10k_firmware_mode mode)
 	if (ret)
 		return ret;
 
-	/* Data transfer is not initiated, when reduced Tx completion
-	 * is used for SDIO. disable it until fixed
-	 */
-	param &= ~HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET;
+	param |= HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET;
 
 	/* Alternate credit size of 1544 as used by SDIO firmware is
 	 * not big enough for mac80211 / native wifi frames. disable it
diff --git a/drivers/net/wireless/ath/ath10k/hif.h b/drivers/net/wireless/ath/ath10k/hif.h
index 496ee34a4d78..0dd8973d0acf 100644
--- a/drivers/net/wireless/ath/ath10k/hif.h
+++ b/drivers/net/wireless/ath/ath10k/hif.h
@@ -56,6 +56,8 @@ struct ath10k_hif_ops {
 
 	int (*swap_mailbox)(struct ath10k *ar);
 
+	int (*get_htt_tx_complete)(struct ath10k *ar);
+
 	int (*map_service_to_pipe)(struct ath10k *ar, u16 service_id,
 				   u8 *ul_pipe, u8 *dl_pipe);
 
@@ -144,6 +146,13 @@ static inline int ath10k_hif_swap_mailbox(struct ath10k *ar)
 	return 0;
 }
 
+static inline int ath10k_hif_get_htt_tx_complete(struct ath10k *ar)
+{
+	if (ar->hif.ops->get_htt_tx_complete)
+		return ar->hif.ops->get_htt_tx_complete(ar);
+	return 0;
+}
+
 static inline int ath10k_hif_map_service_to_pipe(struct ath10k *ar,
 						 u16 service_id,
 						 u8 *ul_pipe, u8 *dl_pipe)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 1d4d1a1992fe..4c6cdc24d282 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -660,6 +660,16 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
 	return 0;
 }
 
+void ath10k_htc_change_tx_credit_flow(struct ath10k_htc *htc,
+				      enum ath10k_htc_ep_id eid,
+				      bool enable)
+{
+	struct ath10k *ar = htc->ar;
+	struct ath10k_htc_ep *ep = &ar->htc.endpoint[eid];
+
+	ep->tx_credit_flow_enabled = enable;
+}
+
 int ath10k_htc_connect_service(struct ath10k_htc *htc,
 			       struct ath10k_htc_svc_conn_req *conn_req,
 			       struct ath10k_htc_svc_conn_resp *conn_resp)
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index f55d3caec61f..d0f37cbdab8b 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -367,6 +367,9 @@ int ath10k_htc_start(struct ath10k_htc *htc);
 int ath10k_htc_connect_service(struct ath10k_htc *htc,
 			       struct ath10k_htc_svc_conn_req  *conn_req,
 			       struct ath10k_htc_svc_conn_resp *conn_resp);
+void ath10k_htc_change_tx_credit_flow(struct ath10k_htc *htc,
+				      enum ath10k_htc_ep_id eid,
+				      bool enable);
 int ath10k_htc_send(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid,
 		    struct sk_buff *packet);
 struct sk_buff *ath10k_htc_alloc_skb(struct ath10k *ar, int size);
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 7b75200ceae5..4354bf285ff1 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -10,6 +10,7 @@
 #include "htt.h"
 #include "core.h"
 #include "debug.h"
+#include "hif.h"
 
 static const enum htt_t2h_msg_type htt_main_t2h_msg_types[] = {
 	[HTT_MAIN_T2H_MSG_TYPE_VERSION_CONF] = HTT_T2H_MSG_TYPE_VERSION_CONF,
@@ -153,6 +154,10 @@ int ath10k_htt_connect(struct ath10k_htt *htt)
 
 	htt->eid = conn_resp.eid;
 
+	htt->disable_tx_comp = ath10k_hif_get_htt_tx_complete(htt->ar);
+	if (htt->disable_tx_comp)
+		ath10k_htc_change_tx_credit_flow(&htt->ar->htc, htt->eid, true);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 30c080094af1..889bf9fe051a 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -150,9 +150,19 @@ enum htt_data_tx_desc_flags1 {
 	HTT_DATA_TX_DESC_FLAGS1_MORE_IN_BATCH    = 1 << 12,
 	HTT_DATA_TX_DESC_FLAGS1_CKSUM_L3_OFFLOAD = 1 << 13,
 	HTT_DATA_TX_DESC_FLAGS1_CKSUM_L4_OFFLOAD = 1 << 14,
-	HTT_DATA_TX_DESC_FLAGS1_RSVD1            = 1 << 15
+	HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE      = 1 << 15
 };
 
+#define HTT_TX_CREDIT_DELTA_ABS_M      0xffff0000
+#define HTT_TX_CREDIT_DELTA_ABS_S      16
+#define HTT_TX_CREDIT_DELTA_ABS_GET(word) \
+	    (((word) & HTT_TX_CREDIT_DELTA_ABS_M) >> HTT_TX_CREDIT_DELTA_ABS_S)
+
+#define HTT_TX_CREDIT_SIGN_BIT_M       0x00000100
+#define HTT_TX_CREDIT_SIGN_BIT_S       8
+#define HTT_TX_CREDIT_SIGN_BIT_GET(word) \
+	    (((word) & HTT_TX_CREDIT_SIGN_BIT_M) >> HTT_TX_CREDIT_SIGN_BIT_S)
+
 enum htt_data_tx_ext_tid {
 	HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 16,
 	HTT_DATA_TX_EXT_TID_MGMT                = 17,
@@ -2019,6 +2029,7 @@ struct ath10k_htt {
 	bool tx_mem_allocated;
 	const struct ath10k_htt_tx_ops *tx_ops;
 	const struct ath10k_htt_rx_ops *rx_ops;
+	bool disable_tx_comp;
 };
 
 struct ath10k_htt_tx_ops {
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index d95b63f133ab..049861f38e95 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -3778,6 +3778,9 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 	}
 	case HTT_T2H_MSG_TYPE_MGMT_TX_COMPLETION: {
 		struct htt_tx_done tx_done = {};
+		struct ath10k_htt *htt = &ar->htt;
+		struct ath10k_htc *htc = &ar->htc;
+		struct ath10k_htc_ep *ep = &ar->htc.endpoint[htt->eid];
 		int status = __le32_to_cpu(resp->mgmt_tx_completion.status);
 		int info = __le32_to_cpu(resp->mgmt_tx_completion.info);
 
@@ -3803,6 +3806,12 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 			break;
 		}
 
+		if (htt->disable_tx_comp) {
+			spin_lock_bh(&htc->tx_lock);
+			ep->tx_credits++;
+			spin_unlock_bh(&htc->tx_lock);
+		}
+
 		status = ath10k_txrx_tx_unref(htt, &tx_done);
 		if (!status) {
 			spin_lock_bh(&htt->tx_lock);
@@ -3877,8 +3886,31 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		skb_queue_tail(&htt->rx_in_ord_compl_q, skb);
 		return false;
 	}
-	case HTT_T2H_MSG_TYPE_TX_CREDIT_UPDATE_IND:
+	case HTT_T2H_MSG_TYPE_TX_CREDIT_UPDATE_IND: {
+		struct ath10k_htt *htt = &ar->htt;
+		struct ath10k_htc *htc = &ar->htc;
+		struct ath10k_htc_ep *ep = &ar->htc.endpoint[htt->eid];
+		u32 msg_word = __le32_to_cpu(*(__le32 *)resp);
+		int htt_credit_delta;
+
+		htt_credit_delta = HTT_TX_CREDIT_DELTA_ABS_GET(msg_word);
+		if (HTT_TX_CREDIT_SIGN_BIT_GET(msg_word))
+			htt_credit_delta = -htt_credit_delta;
+
+		ath10k_dbg(ar, ATH10K_DBG_HTT,
+			   "credit update: delta:%d\n",
+			   htt_credit_delta);
+
+		if (htt->disable_tx_comp) {
+			spin_lock_bh(&htc->tx_lock);
+			ep->tx_credits += htt_credit_delta;
+			spin_unlock_bh(&htc->tx_lock);
+			ath10k_dbg(ar, ATH10K_DBG_HTT,
+				   "credit total:%d\n",
+				   ep->tx_credits);
+		}
 		break;
+	}
 	case HTT_T2H_MSG_TYPE_CHAN_CHANGE: {
 		u32 phymode = __le32_to_cpu(resp->chan_change.phymode);
 		u32 freq = __le32_to_cpu(resp->chan_change.freq);
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index a182c0944cc7..c6c4b2a4d20f 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
 
 void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
 {
+	struct ath10k_htt *htt = &ar->htt;
+	struct htt_tx_done tx_done = {0};
+	struct htt_cmd_hdr *htt_hdr;
+	struct htt_data_tx_desc *desc_hdr;
+	u16 flags1;
+
 	dev_kfree_skb_any(skb);
+
+	if (!htt->disable_tx_comp)
+		return;
+
+	htt_hdr = (struct htt_cmd_hdr *)skb->data;
+	if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM)
+		return;
+
+	desc_hdr = (struct htt_data_tx_desc *)
+		(skb->data + sizeof(*htt_hdr));
+	flags1 = __le16_to_cpu(desc_hdr->flags1);
+
+	ath10k_dbg(ar, ATH10K_DBG_HTT,
+		   "htt tx complete msdu id:%u ,flags1:%x\n",
+		   __le16_to_cpu(desc_hdr->id), flags1);
+
+	if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE)
+		return;
+
+	tx_done.status = HTT_TX_COMPL_STATE_ACK;
+	tx_done.msdu_id = __le16_to_cpu(desc_hdr->id);
+	ath10k_txrx_tx_unref(&ar->htt, &tx_done);
 }
 
 void ath10k_htt_hif_tx_complete(struct ath10k *ar, struct sk_buff *skb)
@@ -1269,6 +1297,9 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
 		flags0 |= SM(ATH10K_HW_TXRX_MGMT,
 			     HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
 		flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
+
+		if (htt->disable_tx_comp)
+			flags1 |= HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE;
 		break;
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 35a362329a4f..2661c6893ca5 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -762,7 +762,7 @@ ath10k_is_rssi_enable(struct ath10k_hw_params *hw,
 #define TARGET_TLV_NUM_TDLS_VDEVS		1
 #define TARGET_TLV_NUM_TIDS			((TARGET_TLV_NUM_PEERS) * 2)
 #define TARGET_TLV_NUM_MSDU_DESC		(1024 + 32)
-#define TARGET_TLV_NUM_MSDU_DESC_HL		64
+#define TARGET_TLV_NUM_MSDU_DESC_HL		1024
 #define TARGET_TLV_NUM_WOW_PATTERNS		22
 #define TARGET_TLV_MGMT_NUM_MSDU_DESC		(50)
 
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 120200a93bcc..84910446df50 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -1674,6 +1674,28 @@ static int ath10k_sdio_hif_swap_mailbox(struct ath10k *ar)
 	return 0;
 }
 
+static int ath10k_sdio_get_htt_tx_complete(struct ath10k *ar)
+{
+	u32 addr, val;
+	int ret;
+
+	addr = host_interest_item_address(HI_ITEM(hi_acs_flags));
+
+	ret = ath10k_sdio_hif_diag_read32(ar, addr, &val);
+	if (ret) {
+		ath10k_warn(ar,
+			    "unable to read hi_acs_flags for htt tx comple : %d\n", ret);
+		return ret;
+	}
+
+	ret = (val & HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_FW_ACK);
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio reduce tx complete fw%sack\n",
+		   ret ? " " : " not ");
+
+	return ret;
+}
+
 /* HIF start/stop */
 
 static int ath10k_sdio_hif_start(struct ath10k *ar)
@@ -1943,6 +1965,7 @@ static const struct ath10k_hif_ops ath10k_sdio_hif_ops = {
 	.start			= ath10k_sdio_hif_start,
 	.stop			= ath10k_sdio_hif_stop,
 	.swap_mailbox		= ath10k_sdio_hif_swap_mailbox,
+	.get_htt_tx_complete	= ath10k_sdio_get_htt_tx_complete,
 	.map_service_to_pipe	= ath10k_sdio_hif_map_service_to_pipe,
 	.get_default_pipe	= ath10k_sdio_hif_get_default_pipe,
 	.send_complete_check	= ath10k_sdio_hif_send_complete_check,
-- 
2.23.0

[PATCH v8 2/4] ath10k: change ATH10K_SDIO_BUS_REQUEST_MAX_NUM from 64 to 1024

[Wen Gong]

sdio bus bandwidth is low, sometimes for high performance TX test,
it will lack of ath10k_sdio_bus_request, it will print message:
ath10k_sdio mmc1:0001:1: unable to allocate bus request for async request

change the num from 64 to 1024 will not happen it.

Tested with QCA6174 SDIO with firmware
WLAN.RMH.4.4.1-00017-QCARMSWP-1.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/sdio.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index b8c7ac0330bd..556f67b0f89e 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -37,7 +37,7 @@
 	(ATH10K_SDIO_MAX_BUFFER_SIZE - sizeof(struct ath10k_htc_hdr))
 
 #define ATH10K_HIF_MBOX_NUM_MAX                 4
-#define ATH10K_SDIO_BUS_REQUEST_MAX_NUM         64
+#define ATH10K_SDIO_BUS_REQUEST_MAX_NUM         1024
 
 #define ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ (100 * HZ)
 
@@ -98,6 +98,7 @@
 #define ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_OFF 0xFFFEFFFF
 #define ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_ON 0x10000
 
+/* TODO: remove this and use skb->cb instead, much cleaner approach */
 struct ath10k_sdio_bus_request {
 	struct list_head list;
 
-- 
2.23.0

[PATCH v8 4/4] ath10k: enable alt data of TX path for sdio

[Wen Gong]

The default credit size is 1792 bytes, but the IP mtu is 1500 bytes,
then it has about 290 bytes's waste for each data packet on sdio
transfer path for TX bundle, it will reduce the transmission utilization
ratio for data packet.

This patch enable the small credit size in firmware, firmware will use
the new credit size 1556 bytes, it will increase the transmission
utilization ratio for data packet on TX patch. It results in significant
performance improvement on TX path.

This patch only effect sdio chip, it will not effect PCI, SNOC etc.

Tested with QCA6174 SDIO with firmware
WLAN.RMH.4.4.1-00017-QCARMSWP-1.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c |  8 ++++----
 drivers/net/wireless/ath/ath10k/htc.c  | 12 ++++++++++--
 drivers/net/wireless/ath/ath10k/htc.h  | 13 +++++++++++--
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index fd35c7c93c30..f31203434997 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -697,10 +697,10 @@ static int ath10k_init_sdio(struct ath10k *ar, enum ath10k_firmware_mode mode)
 
 	param |= HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET;
 
-	/* Alternate credit size of 1544 as used by SDIO firmware is
-	 * not big enough for mac80211 / native wifi frames. disable it
-	 */
-	param &= ~HI_ACS_FLAGS_ALT_DATA_CREDIT_SIZE;
+	if (mode == ATH10K_FIRMWARE_MODE_NORMAL)
+		param |= HI_ACS_FLAGS_ALT_DATA_CREDIT_SIZE;
+	else
+		param &= ~HI_ACS_FLAGS_ALT_DATA_CREDIT_SIZE;
 
 	if (mode == ATH10K_FIRMWARE_MODE_UTF)
 		param &= ~HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_SET;
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 8eac8cbc606c..995bc0236c43 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -957,12 +957,16 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
 	 */
 	if (htc->control_resp_len >=
 	    sizeof(msg->hdr) + sizeof(msg->ready_ext)) {
+		htc->alt_data_credit_size =
+			__le16_to_cpu(msg->ready_ext.reserved) &
+			ATH10K_HTC_MSG_READY_EXT_ALT_DATA_MASK;
 		htc->max_msgs_per_htc_bundle =
 			min_t(u8, msg->ready_ext.max_msgs_per_htc_bundle,
 			      HTC_HOST_MAX_MSG_PER_RX_BUNDLE);
 		ath10k_dbg(ar, ATH10K_DBG_HTC,
-			   "Extended ready message. RX bundle size: %d\n",
-			   htc->max_msgs_per_htc_bundle);
+			   "Extended ready message. RX bundle size: %d, alt size:%d\n",
+			   htc->max_msgs_per_htc_bundle,
+			   htc->alt_data_credit_size);
 	}
 
 	INIT_WORK(&ar->bundle_tx_work, ath10k_htc_bundle_tx_work);
@@ -1114,6 +1118,10 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
 	ep->tx_credits = tx_alloc;
 	ep->tx_credit_size = htc->target_credit_size;
 
+	if (conn_req->service_id == ATH10K_HTC_SVC_ID_HTT_DATA_MSG &&
+	    htc->alt_data_credit_size != 0)
+		ep->tx_credit_size = htc->alt_data_credit_size;
+
 	/* copy all the callbacks */
 	ep->ep_ops = conn_req->ep_ops;
 
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 77ec6499f9c3..337eb3f86b8d 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -100,6 +100,8 @@ enum ath10k_htc_conn_flags {
 #define ATH10K_HTC_CONN_FLAGS_RECV_ALLOC_LSB  8
 };
 
+#define ATH10K_HTC_MSG_READY_EXT_ALT_DATA_MASK 0xFFF
+
 enum ath10k_htc_conn_svc_status {
 	ATH10K_HTC_CONN_SVC_STATUS_SUCCESS      = 0,
 	ATH10K_HTC_CONN_SVC_STATUS_NOT_FOUND    = 1,
@@ -136,8 +138,14 @@ struct ath10k_htc_ready_extended {
 	struct ath10k_htc_ready base;
 	u8 htc_version; /* @enum ath10k_htc_version */
 	u8 max_msgs_per_htc_bundle;
-	u8 pad0;
-	u8 pad1;
+	union {
+		__le16 reserved;
+		struct {
+			u8 pad0;
+			u8 pad1;
+		} __packed;
+	} __packed;
+
 } __packed;
 
 struct ath10k_htc_conn_svc {
@@ -374,6 +382,7 @@ struct ath10k_htc {
 	int total_transmit_credits;
 	int target_credit_size;
 	u8 max_msgs_per_htc_bundle;
+	int alt_data_credit_size;
 };
 
 int ath10k_htc_init(struct ath10k *ar);
-- 
2.23.0

[PATCH v8 3/4] ath10k: add htt TX bundle for sdio

[Wen Gong]

The transmission utilization ratio for sdio bus for small packet is
slow, because the space and time cost for sdio bus is same for large
length packet and small length packet. So the speed of data for large
length packet is higher than small length.

Test result of different length of data:

data packet(byte)   cost time(us)   calculated rate(Mbps)
      256               28                73
      512               33               124
     1024               35               234
     1792               45               318
    14336              168               682
    28672              333               688
    57344              660               695

This patch change the TX packet from single packet to a large length
bundle packet, max size is 32, it results in significant performance
improvement on TX path.

This patch only effect sdio chip, it will not effect PCI, SNOC etc.
It only enable bundle for sdio chip.

Tested with QCA6174 SDIO with firmware
WLAN.RMH.4.4.1-00017-QCARMSWP-1.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c   |  14 +-
 drivers/net/wireless/ath/ath10k/core.h   |   4 +-
 drivers/net/wireless/ath/ath10k/htc.c    | 372 +++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/htc.h    |  24 +-
 drivers/net/wireless/ath/ath10k/htt.c    |   8 +
 drivers/net/wireless/ath/ath10k/htt.h    |   4 +
 drivers/net/wireless/ath/ath10k/htt_rx.c |   1 +
 drivers/net/wireless/ath/ath10k/htt_tx.c |   8 +-
 8 files changed, 398 insertions(+), 37 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5a49edfdf9e6..fd35c7c93c30 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -3219,6 +3219,11 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 	if (!ar->workqueue_aux)
 		goto err_free_wq;
 
+	ar->workqueue_tx_complete =
+		create_singlethread_workqueue("ath10k_tx_complete_wq");
+	if (!ar->workqueue_tx_complete)
+		goto err_free_aux_wq;
+
 	mutex_init(&ar->conf_mutex);
 	mutex_init(&ar->dump_mutex);
 	spin_lock_init(&ar->data_lock);
@@ -3244,7 +3249,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 
 	ret = ath10k_coredump_create(ar);
 	if (ret)
-		goto err_free_aux_wq;
+		goto err_free_tx_complete;
 
 	ret = ath10k_debug_create(ar);
 	if (ret)
@@ -3254,12 +3259,12 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 
 err_free_coredump:
 	ath10k_coredump_destroy(ar);
-
+err_free_tx_complete:
+	destroy_workqueue(ar->workqueue_tx_complete);
 err_free_aux_wq:
 	destroy_workqueue(ar->workqueue_aux);
 err_free_wq:
 	destroy_workqueue(ar->workqueue);
-
 err_free_mac:
 	ath10k_mac_destroy(ar);
 
@@ -3275,6 +3280,9 @@ void ath10k_core_destroy(struct ath10k *ar)
 	flush_workqueue(ar->workqueue_aux);
 	destroy_workqueue(ar->workqueue_aux);
 
+	flush_workqueue(ar->workqueue_tx_complete);
+	destroy_workqueue(ar->workqueue_tx_complete);
+
 	ath10k_debug_destroy(ar);
 	ath10k_coredump_destroy(ar);
 	ath10k_htt_tx_destroy(&ar->htt);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index af68eb5d0776..d6d13654c385 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1088,7 +1088,7 @@ struct ath10k {
 	struct workqueue_struct *workqueue;
 	/* Auxiliary workqueue */
 	struct workqueue_struct *workqueue_aux;
-
+	struct workqueue_struct *workqueue_tx_complete;
 	/* prevents concurrent FW reconfiguration */
 	struct mutex conf_mutex;
 
@@ -1129,6 +1129,8 @@ struct ath10k {
 
 	struct work_struct register_work;
 	struct work_struct restart_work;
+	struct work_struct bundle_tx_work;
+	struct work_struct tx_complete_work;
 
 	/* cycle count is reported twice for each visited channel during scan.
 	 * access protected by data_lock
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 4c6cdc24d282..8eac8cbc606c 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -51,10 +51,12 @@ void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
 				     struct sk_buff *skb)
 {
 	struct ath10k *ar = ep->htc->ar;
+	struct ath10k_htc_hdr *hdr;
 
 	ath10k_dbg(ar, ATH10K_DBG_HTC, "%s: ep %d skb %pK\n", __func__,
 		   ep->eid, skb);
 
+	hdr = (struct ath10k_htc_hdr *)skb->data;
 	ath10k_htc_restore_tx_skb(ep->htc, skb);
 
 	if (!ep->ep_ops.ep_tx_complete) {
@@ -63,6 +65,11 @@ void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
 		return;
 	}
 
+	if (hdr->flags & ATH10K_HTC_FLAG_SEND_BUNDLE) {
+		dev_kfree_skb_any(skb);
+		return;
+	}
+
 	ep->ep_ops.ep_tx_complete(ep->htc->ar, skb);
 }
 EXPORT_SYMBOL(ath10k_htc_notify_tx_completion);
@@ -78,7 +85,7 @@ static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep,
 	hdr->eid = ep->eid;
 	hdr->len = __cpu_to_le16(skb->len - sizeof(*hdr));
 	hdr->flags = 0;
-	if (ep->tx_credit_flow_enabled)
+	if (ep->tx_credit_flow_enabled && !ep->bundle_tx)
 		hdr->flags |= ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE;
 
 	spin_lock_bh(&ep->htc->tx_lock);
@@ -86,6 +93,63 @@ static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep,
 	spin_unlock_bh(&ep->htc->tx_lock);
 }
 
+static int ath10k_htc_consume_credit(struct ath10k_htc_ep *ep,
+				     unsigned int len,
+				     bool consume)
+{
+	struct ath10k_htc *htc = ep->htc;
+	struct ath10k *ar = htc->ar;
+	enum ath10k_htc_ep_id eid = ep->eid;
+	int credits, ret = 0;
+
+	if (!ep->tx_credit_flow_enabled)
+		return 0;
+
+	credits = DIV_ROUND_UP(len, ep->tx_credit_size);
+	spin_lock_bh(&htc->tx_lock);
+
+	if (ep->tx_credits < credits) {
+		ath10k_dbg(ar, ATH10K_DBG_HTC,
+			   "htc insufficient credits ep %d required %d available %d consume %d\n",
+			   eid, credits, ep->tx_credits, consume);
+		ret = -EAGAIN;
+		goto unlock;
+	}
+
+	if (consume) {
+		ep->tx_credits -= credits;
+		ath10k_dbg(ar, ATH10K_DBG_HTC,
+			   "htc ep %d consumed %d credits (total %d)\n",
+			   eid, credits, ep->tx_credits);
+	}
+
+unlock:
+	spin_unlock_bh(&htc->tx_lock);
+	return ret;
+}
+
+static void ath10k_htc_release_credit(struct ath10k_htc_ep *ep, unsigned int len)
+{
+	struct ath10k_htc *htc = ep->htc;
+	struct ath10k *ar = htc->ar;
+	enum ath10k_htc_ep_id eid = ep->eid;
+	int credits;
+
+	if (!ep->tx_credit_flow_enabled)
+		return;
+
+	credits = DIV_ROUND_UP(len, ep->tx_credit_size);
+	spin_lock_bh(&htc->tx_lock);
+	ep->tx_credits += credits;
+	ath10k_dbg(ar, ATH10K_DBG_HTC,
+		   "htc ep %d reverted %d credits back (total %d)\n",
+		   eid, credits, ep->tx_credits);
+	spin_unlock_bh(&htc->tx_lock);
+
+	if (ep->ep_ops.ep_tx_credits)
+		ep->ep_ops.ep_tx_credits(htc->ar);
+}
+
 int ath10k_htc_send(struct ath10k_htc *htc,
 		    enum ath10k_htc_ep_id eid,
 		    struct sk_buff *skb)
@@ -95,8 +159,8 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 	struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
 	struct ath10k_hif_sg_item sg_item;
 	struct device *dev = htc->ar->dev;
-	int credits = 0;
 	int ret;
+	unsigned int skb_len;
 
 	if (htc->ar->state == ATH10K_STATE_WEDGED)
 		return -ECOMM;
@@ -108,23 +172,10 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 
 	skb_push(skb, sizeof(struct ath10k_htc_hdr));
 
-	if (ep->tx_credit_flow_enabled) {
-		credits = DIV_ROUND_UP(skb->len, htc->target_credit_size);
-		spin_lock_bh(&htc->tx_lock);
-		if (ep->tx_credits < credits) {
-			ath10k_dbg(ar, ATH10K_DBG_HTC,
-				   "htc insufficient credits ep %d required %d available %d\n",
-				   eid, credits, ep->tx_credits);
-			spin_unlock_bh(&htc->tx_lock);
-			ret = -EAGAIN;
-			goto err_pull;
-		}
-		ep->tx_credits -= credits;
-		ath10k_dbg(ar, ATH10K_DBG_HTC,
-			   "htc ep %d consumed %d credits (total %d)\n",
-			   eid, credits, ep->tx_credits);
-		spin_unlock_bh(&htc->tx_lock);
-	}
+	skb_len = skb->len;
+	ret = ath10k_htc_consume_credit(ep, skb_len, true);
+	if (ret)
+		goto err_pull;
 
 	ath10k_htc_prepare_tx_skb(ep, skb);
 
@@ -155,17 +206,7 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 	if (ar->bus_param.dev_type != ATH10K_DEV_TYPE_HL)
 		dma_unmap_single(dev, skb_cb->paddr, skb->len, DMA_TO_DEVICE);
 err_credits:
-	if (ep->tx_credit_flow_enabled) {
-		spin_lock_bh(&htc->tx_lock);
-		ep->tx_credits += credits;
-		ath10k_dbg(ar, ATH10K_DBG_HTC,
-			   "htc ep %d reverted %d credits back (total %d)\n",
-			   eid, credits, ep->tx_credits);
-		spin_unlock_bh(&htc->tx_lock);
-
-		if (ep->ep_ops.ep_tx_credits)
-			ep->ep_ops.ep_tx_credits(htc->ar);
-	}
+	ath10k_htc_release_credit(ep, skb_len);
 err_pull:
 	skb_pull(skb, sizeof(struct ath10k_htc_hdr));
 	return ret;
@@ -581,6 +622,273 @@ static u8 ath10k_htc_get_credit_allocation(struct ath10k_htc *htc,
 	return allocation;
 }
 
+static int ath10k_htc_send_bundle(struct ath10k_htc_ep *ep,
+				  struct sk_buff *bundle_skb,
+				  struct sk_buff_head *tx_save_head)
+{
+	struct ath10k_hif_sg_item sg_item;
+	struct ath10k_htc *htc = ep->htc;
+	struct ath10k *ar = htc->ar;
+	struct sk_buff *skb;
+	int ret, cn = 0;
+	unsigned int skb_len;
+
+	ath10k_dbg(ar, ATH10K_DBG_HTC, "bundle skb: len:%d\n", bundle_skb->len);
+	skb_len = bundle_skb->len;
+	ret = ath10k_htc_consume_credit(ep, skb_len, true);
+
+	if (!ret) {
+		sg_item.transfer_id = ep->eid;
+		sg_item.transfer_context = bundle_skb;
+		sg_item.vaddr = bundle_skb->data;
+		sg_item.len = bundle_skb->len;
+
+		ret = ath10k_hif_tx_sg(htc->ar, ep->ul_pipe_id, &sg_item, 1);
+		if (ret)
+			ath10k_htc_release_credit(ep, skb_len);
+	}
+
+	if (ret)
+		dev_kfree_skb_any(bundle_skb);
+
+	for (cn = 0; (skb = skb_dequeue_tail(tx_save_head)); cn++) {
+		if (ret) {
+			skb_pull(skb, sizeof(struct ath10k_htc_hdr));
+			skb_queue_head(&ep->tx_req_head, skb);
+		} else {
+			skb_queue_tail(&ep->tx_complete_head, skb);
+		}
+	}
+
+	if (!ret)
+		queue_work(ar->workqueue_tx_complete, &ar->tx_complete_work);
+
+	ath10k_dbg(ar, ATH10K_DBG_HTC,
+		   "bundle tx status:%d, eid:%d, req count:%d, count:%d, len:%d\n",
+		   ret, ep->eid, skb_queue_len(&ep->tx_req_head), cn, bundle_skb->len);
+	return ret;
+}
+
+static void ath10k_htc_send_one_skb(struct ath10k_htc_ep *ep, struct sk_buff *skb)
+{
+	struct ath10k_htc *htc = ep->htc;
+	struct ath10k *ar = htc->ar;
+	int ret;
+
+	ret = ath10k_htc_send(htc, ep->eid, skb);
+
+	if (ret)
+		skb_queue_head(&ep->tx_req_head, skb);
+
+	ath10k_dbg(ar, ATH10K_DBG_HTC, "tx one status:%d, eid:%d, len:%d, pending count:%d\n",
+		   ret, ep->eid, skb->len, skb_queue_len(&ep->tx_req_head));
+}
+
+static int ath10k_htc_send_bundle_skbs(struct ath10k_htc_ep *ep)
+{
+	struct ath10k_htc *htc = ep->htc;
+	struct sk_buff *bundle_skb, *skb;
+	struct sk_buff_head tx_save_head;
+	struct ath10k_htc_hdr *hdr;
+	u8 *bundle_buf;
+	int ret = 0, credit_pad, credit_remainder, trans_len, bundles_left = 0;
+
+	if (htc->ar->state == ATH10K_STATE_WEDGED)
+		return -ECOMM;
+
+	if (ep->tx_credit_flow_enabled &&
+	    ep->tx_credits < ATH10K_MIN_CREDIT_PER_HTC_TX_BUNDLE)
+		return 0;
+
+	bundles_left = ATH10K_MAX_MSG_PER_HTC_TX_BUNDLE * ep->tx_credit_size;
+	bundle_skb = dev_alloc_skb(bundles_left);
+
+	if (!bundle_skb)
+		return -ENOMEM;
+
+	bundle_buf = bundle_skb->data;
+	skb_queue_head_init(&tx_save_head);
+
+	while (true) {
+		skb = skb_dequeue(&ep->tx_req_head);
+		if (!skb)
+			break;
+
+		credit_pad = 0;
+		trans_len = skb->len + sizeof(*hdr);
+		credit_remainder = trans_len % ep->tx_credit_size;
+
+		if (credit_remainder != 0) {
+			credit_pad = ep->tx_credit_size - credit_remainder;
+			trans_len += credit_pad;
+		}
+
+		ret = ath10k_htc_consume_credit(ep,
+						bundle_buf + trans_len - bundle_skb->data,
+						false);
+		if (ret) {
+			skb_queue_head(&ep->tx_req_head, skb);
+			break;
+		}
+
+		if (bundles_left < trans_len) {
+			bundle_skb->len = bundle_buf - bundle_skb->data;
+			ret = ath10k_htc_send_bundle(ep, bundle_skb, &tx_save_head);
+
+			if (ret) {
+				skb_queue_head(&ep->tx_req_head, skb);
+				return ret;
+			}
+
+			if (skb_queue_len(&ep->tx_req_head) == 0) {
+				ath10k_htc_send_one_skb(ep, skb);
+				return ret;
+			}
+
+			if (ep->tx_credit_flow_enabled &&
+			    ep->tx_credits < ATH10K_MIN_CREDIT_PER_HTC_TX_BUNDLE) {
+				skb_queue_head(&ep->tx_req_head, skb);
+				return 0;
+			}
+
+			bundles_left =
+				ATH10K_MAX_MSG_PER_HTC_TX_BUNDLE * ep->tx_credit_size;
+			bundle_skb = dev_alloc_skb(bundles_left);
+
+			if (!bundle_skb) {
+				skb_queue_head(&ep->tx_req_head, skb);
+				return -ENOMEM;
+			}
+			bundle_buf = bundle_skb->data;
+			skb_queue_head_init(&tx_save_head);
+		}
+
+		skb_push(skb, sizeof(struct ath10k_htc_hdr));
+		ath10k_htc_prepare_tx_skb(ep, skb);
+
+		memcpy(bundle_buf, skb->data, skb->len);
+		hdr = (struct ath10k_htc_hdr *)bundle_buf;
+		hdr->flags |= ATH10K_HTC_FLAG_SEND_BUNDLE;
+		hdr->pad_len = __cpu_to_le16(credit_pad);
+		bundle_buf += trans_len;
+		bundles_left -= trans_len;
+		skb_queue_tail(&tx_save_head, skb);
+	}
+
+	if (bundle_buf != bundle_skb->data) {
+		bundle_skb->len = bundle_buf - bundle_skb->data;
+		ret = ath10k_htc_send_bundle(ep, bundle_skb, &tx_save_head);
+	} else {
+		dev_kfree_skb_any(bundle_skb);
+	}
+
+	return ret;
+}
+
+static void ath10k_htc_bundle_tx_work(struct work_struct *work)
+{
+	struct ath10k *ar = container_of(work, struct ath10k, bundle_tx_work);
+	struct ath10k_htc_ep *ep;
+	struct sk_buff *skb;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ar->htc.endpoint); i++) {
+		ep = &ar->htc.endpoint[i];
+
+		if (!ep->bundle_tx)
+			continue;
+
+		ath10k_dbg(ar, ATH10K_DBG_HTC, "bundle tx work, eid:%d, count:%d\n",
+			   ep->eid, skb_queue_len(&ep->tx_req_head));
+
+		if (skb_queue_len(&ep->tx_req_head) >=
+		    ATH10K_MIN_MSG_PER_HTC_TX_BUNDLE) {
+			ath10k_htc_send_bundle_skbs(ep);
+		} else {
+			skb = skb_dequeue(&ep->tx_req_head);
+
+			if (!skb)
+				continue;
+			ath10k_htc_send_one_skb(ep, skb);
+		}
+	}
+}
+
+static void ath10k_htc_tx_complete_work(struct work_struct *work)
+{
+	struct ath10k *ar = container_of(work, struct ath10k, tx_complete_work);
+	struct ath10k_htc_ep *ep;
+	enum ath10k_htc_ep_id eid;
+	struct sk_buff *skb;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ar->htc.endpoint); i++) {
+		ep = &ar->htc.endpoint[i];
+		eid = ep->eid;
+		if (ep->bundle_tx && eid == ar->htt.eid) {
+			ath10k_dbg(ar, ATH10K_DBG_HTC, "bundle tx complete, eid:%d, pending complete count:%d\n",
+				   ep->eid, skb_queue_len(&ep->tx_complete_head));
+
+			while (true) {
+				skb = skb_dequeue(&ep->tx_complete_head);
+				if (!skb)
+					break;
+				ath10k_htc_notify_tx_completion(ep, skb);
+			}
+		}
+	}
+}
+
+int ath10k_htc_send_hl(struct ath10k_htc *htc,
+		       enum ath10k_htc_ep_id eid,
+		       struct sk_buff *skb)
+{
+	struct ath10k_htc_ep *ep = &htc->endpoint[eid];
+	struct ath10k *ar = htc->ar;
+
+	ath10k_dbg(ar, ATH10K_DBG_HTC, "htc send hl: eid:%d, bundle:%d, tx count:%d, len:%d\n",
+		   eid, ep->bundle_tx, skb_queue_len(&ep->tx_req_head), skb->len);
+
+	if (ep->bundle_tx) {
+		skb_queue_tail(&ep->tx_req_head, skb);
+		queue_work(ar->workqueue, &ar->bundle_tx_work);
+		return 0;
+	} else {
+		return ath10k_htc_send(htc, eid, skb);
+	}
+}
+
+void ath10k_htc_setup_tx_req(struct ath10k_htc_ep *ep)
+{
+	if (ep->htc->max_msgs_per_htc_bundle >= ATH10K_MIN_MSG_PER_HTC_TX_BUNDLE &&
+	    !ep->bundle_tx) {
+		ep->bundle_tx = true;
+		skb_queue_head_init(&ep->tx_req_head);
+		skb_queue_head_init(&ep->tx_complete_head);
+	}
+}
+
+void ath10k_htc_stop_hl(struct ath10k *ar)
+{
+	struct ath10k_htc_ep *ep;
+	int i;
+
+	cancel_work_sync(&ar->bundle_tx_work);
+	cancel_work_sync(&ar->tx_complete_work);
+
+	for (i = 0; i < ARRAY_SIZE(ar->htc.endpoint); i++) {
+		ep = &ar->htc.endpoint[i];
+
+		if (!ep->bundle_tx)
+			continue;
+
+		ath10k_dbg(ar, ATH10K_DBG_HTC, "stop tx work, eid:%d, count:%d\n",
+			   ep->eid, skb_queue_len(&ep->tx_req_head));
+
+		skb_queue_purge(&ep->tx_req_head);
+	}
+}
+
 int ath10k_htc_wait_target(struct ath10k_htc *htc)
 {
 	struct ath10k *ar = htc->ar;
@@ -657,6 +965,9 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
 			   htc->max_msgs_per_htc_bundle);
 	}
 
+	INIT_WORK(&ar->bundle_tx_work, ath10k_htc_bundle_tx_work);
+	INIT_WORK(&ar->tx_complete_work, ath10k_htc_tx_complete_work);
+
 	return 0;
 }
 
@@ -801,6 +1112,7 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
 	ep->max_tx_queue_depth = conn_req->max_send_queue_depth;
 	ep->max_ep_message_len = __le16_to_cpu(resp_msg->max_msg_size);
 	ep->tx_credits = tx_alloc;
+	ep->tx_credit_size = htc->target_credit_size;
 
 	/* copy all the callbacks */
 	ep->ep_ops = conn_req->ep_ops;
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index d0f37cbdab8b..77ec6499f9c3 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -64,8 +64,14 @@ struct ath10k_htc_hdr {
 		u8 seq_no; /* for tx */
 		u8 control_byte1;
 	} __packed;
-	u8 pad0;
-	u8 pad1;
+	union {
+		__le16 pad_len;
+		struct {
+			u8 pad0;
+			u8 pad1;
+		} __packed;
+	} __packed;
+
 } __packed __aligned(4);
 
 enum ath10k_ath10k_htc_msg_id {
@@ -102,6 +108,10 @@ enum ath10k_htc_conn_svc_status {
 	ATH10K_HTC_CONN_SVC_STATUS_NO_MORE_EP   = 4
 };
 
+#define ATH10K_MAX_MSG_PER_HTC_TX_BUNDLE        32
+#define ATH10K_MIN_MSG_PER_HTC_TX_BUNDLE        2
+#define ATH10K_MIN_CREDIT_PER_HTC_TX_BUNDLE     2
+
 enum ath10k_htc_setup_complete_flags {
 	ATH10K_HTC_SETUP_COMPLETE_FLAGS_RX_BNDL_EN = 1
 };
@@ -334,7 +344,12 @@ struct ath10k_htc_ep {
 
 	u8 seq_no; /* for debugging */
 	int tx_credits;
+	int tx_credit_size;
 	bool tx_credit_flow_enabled;
+	bool bundle_tx;
+	struct sk_buff_head tx_req_head;
+	struct sk_buff_head tx_complete_head;
+
 };
 
 struct ath10k_htc_svc_tx_credits {
@@ -363,6 +378,7 @@ struct ath10k_htc {
 
 int ath10k_htc_init(struct ath10k *ar);
 int ath10k_htc_wait_target(struct ath10k_htc *htc);
+void ath10k_htc_setup_tx_req(struct ath10k_htc_ep *ep);
 int ath10k_htc_start(struct ath10k_htc *htc);
 int ath10k_htc_connect_service(struct ath10k_htc *htc,
 			       struct ath10k_htc_svc_conn_req  *conn_req,
@@ -372,6 +388,10 @@ void ath10k_htc_change_tx_credit_flow(struct ath10k_htc *htc,
 				      bool enable);
 int ath10k_htc_send(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid,
 		    struct sk_buff *packet);
+void ath10k_htc_stop_hl(struct ath10k *ar);
+
+int ath10k_htc_send_hl(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid,
+		       struct sk_buff *packet);
 struct sk_buff *ath10k_htc_alloc_skb(struct ath10k *ar, int size);
 void ath10k_htc_tx_completion_handler(struct ath10k *ar, struct sk_buff *skb);
 void ath10k_htc_rx_completion_handler(struct ath10k *ar, struct sk_buff *skb);
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 4354bf285ff1..127b4e4980ef 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -135,6 +135,8 @@ int ath10k_htt_connect(struct ath10k_htt *htt)
 {
 	struct ath10k_htc_svc_conn_req conn_req;
 	struct ath10k_htc_svc_conn_resp conn_resp;
+	struct ath10k *ar = htt->ar;
+	struct ath10k_htc_ep *ep;
 	int status;
 
 	memset(&conn_req, 0, sizeof(conn_req));
@@ -142,6 +144,7 @@ int ath10k_htt_connect(struct ath10k_htt *htt)
 
 	conn_req.ep_ops.ep_tx_complete = ath10k_htt_htc_tx_complete;
 	conn_req.ep_ops.ep_rx_complete = ath10k_htt_htc_t2h_msg_handler;
+	conn_req.ep_ops.ep_tx_credits = ath10k_htt_op_ep_tx_credits;
 
 	/* connect to control service */
 	conn_req.service_id = ATH10K_HTC_SVC_ID_HTT_DATA_MSG;
@@ -154,6 +157,11 @@ int ath10k_htt_connect(struct ath10k_htt *htt)
 
 	htt->eid = conn_resp.eid;
 
+	if (ar->bus_param.dev_type == ATH10K_DEV_TYPE_HL) {
+		ep = &ar->htc.endpoint[htt->eid];
+		ath10k_htc_setup_tx_req(ep);
+	}
+
 	htt->disable_tx_comp = ath10k_hif_get_htt_tx_complete(htt->ar);
 	if (htt->disable_tx_comp)
 		ath10k_htc_change_tx_credit_flow(&htt->ar->htc, htt->eid, true);
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 889bf9fe051a..4851a2e3345f 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -2030,6 +2030,9 @@ struct ath10k_htt {
 	const struct ath10k_htt_tx_ops *tx_ops;
 	const struct ath10k_htt_rx_ops *rx_ops;
 	bool disable_tx_comp;
+	bool bundle_tx;
+	struct sk_buff_head tx_req_head;
+	struct sk_buff_head tx_complete_head;
 };
 
 struct ath10k_htt_tx_ops {
@@ -2276,6 +2279,7 @@ int ath10k_htt_tx_fetch_resp(struct ath10k *ar,
 			     __le16 fetch_seq_num,
 			     struct htt_tx_fetch_record *records,
 			     size_t num_records);
+void ath10k_htt_op_ep_tx_credits(struct ath10k *ar);
 
 void ath10k_htt_tx_txq_update(struct ieee80211_hw *hw,
 			      struct ieee80211_txq *txq);
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 049861f38e95..5edbd3bbb7e8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -3908,6 +3908,7 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 			ath10k_dbg(ar, ATH10K_DBG_HTT,
 				   "credit total:%d\n",
 				   ep->tx_credits);
+			ep->ep_ops.ep_tx_credits(htc->ar);
 		}
 		break;
 	}
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index c6c4b2a4d20f..51f060a00b95 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -531,6 +531,7 @@ void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
 
 void ath10k_htt_tx_stop(struct ath10k_htt *htt)
 {
+	ath10k_htc_stop_hl(htt->ar);
 	idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
 	idr_destroy(&htt->pending_tx);
 }
@@ -541,6 +542,11 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
 	ath10k_htt_tx_destroy(htt);
 }
 
+void ath10k_htt_op_ep_tx_credits(struct ath10k *ar)
+{
+	queue_work(ar->workqueue, &ar->bundle_tx_work);
+}
+
 void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct ath10k_htt *htt = &ar->htt;
@@ -1365,7 +1371,7 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
 	 */
 	tx_desc->peerid = __cpu_to_le32(HTT_INVALID_PEERID);
 
-	res = ath10k_htc_send(&htt->ar->htc, htt->eid, msdu);
+	res = ath10k_htc_send_hl(&htt->ar->htc, htt->eid, msdu);
 
 out:
 	return res;
-- 
2.23.0

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Pi-Hsun Shih]

Hi,

On 11/28/19 6:30 PM, Wen Gong wrote:
> ...
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index a182c0944cc7..c6c4b2a4d20f 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
>   
>   void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
>   {
> +	struct ath10k_htt *htt = &ar->htt;
> +	struct htt_tx_done tx_done = {0};
> +	struct htt_cmd_hdr *htt_hdr;
> +	struct htt_data_tx_desc *desc_hdr;
> +	u16 flags1;
> +
>   	dev_kfree_skb_any(skb);
> +
> +	if (!htt->disable_tx_comp)
> +		return;
> +
> +	htt_hdr = (struct htt_cmd_hdr *)skb->data;

skb is already freed on the above line (dev_kfree_skb_any) but is still 
used here, should the dev_kfree_skb_any be moved to the end of this 
function?

> +	if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM)
> +		return;
> +
> +	desc_hdr = (struct htt_data_tx_desc *)
> +		(skb->data + sizeof(*htt_hdr));
> +	flags1 = __le16_to_cpu(desc_hdr->flags1);
> +
> +	ath10k_dbg(ar, ATH10K_DBG_HTT,
> +		   "htt tx complete msdu id:%u ,flags1:%x\n",
> +		   __le16_to_cpu(desc_hdr->id), flags1);
> +
> +	if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE)
> +		return;
> +
> +	tx_done.status = HTT_TX_COMPL_STATE_ACK;
> +	tx_done.msdu_id = __le16_to_cpu(desc_hdr->id);
> +	ath10k_txrx_tx_unref(&ar->htt, &tx_done);
>   }
>   

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Wen Gong]

On 2020-02-11 15:03, Pi-Hsun Shih wrote:
> Hi,
> 
> On 11/28/19 6:30 PM, Wen Gong wrote:
>> ...
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c 
>> b/drivers/net/wireless/ath/ath10k/htt_tx.c
>> index a182c0944cc7..c6c4b2a4d20f 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
>> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
>>     void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff 
>> *skb)
>>   {
>> +	struct ath10k_htt *htt = &ar->htt;
>> +	struct htt_tx_done tx_done = {0};
>> +	struct htt_cmd_hdr *htt_hdr;
>> +	struct htt_data_tx_desc *desc_hdr;
>> +	u16 flags1;
>> +
>>   	dev_kfree_skb_any(skb);
>> +
>> +	if (!htt->disable_tx_comp)
>> +		return;
>> +
>> +	htt_hdr = (struct htt_cmd_hdr *)skb->data;
> 
> skb is already freed on the above line (dev_kfree_skb_any) but is
> still used here, should the dev_kfree_skb_any be moved to the end of
> this function?
> 
skb will not freed on the above line, please see this patch
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Pi-Hsun Shih]

On Tue, Feb 11, 2020 at 5:46 PM Wen Gong <wgong@codeaurora.org> wrote:
>
> On 2020-02-11 15:03, Pi-Hsun Shih wrote:
> > Hi,
> >
> > On 11/28/19 6:30 PM, Wen Gong wrote:
> >> ...
> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> b/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> index a182c0944cc7..c6c4b2a4d20f 100644
> >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
> >>     void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff
> >> *skb)
> >>   {
> >> +    struct ath10k_htt *htt = &ar->htt;
> >> +    struct htt_tx_done tx_done = {0};
> >> +    struct htt_cmd_hdr *htt_hdr;
> >> +    struct htt_data_tx_desc *desc_hdr;
> >> +    u16 flags1;
> >> +
> >>      dev_kfree_skb_any(skb);
> >> +
> >> +    if (!htt->disable_tx_comp)
> >> +            return;
> >> +
> >> +    htt_hdr = (struct htt_cmd_hdr *)skb->data;
> >
> > skb is already freed on the above line (dev_kfree_skb_any) but is
> > still used here, should the dev_kfree_skb_any be moved to the end of
> > this function?
> >
> skb will not freed on the above line, please see this patch
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a

IIUC the commit only makes the skb not freed in ieee80211_tx_status,
but it's still freed in ath10k_htt_htc_tx_complete (by
dev_kfree_skb_any)?

While booting with this patch (and the
30382dd1cf3a141bfaa568ee183c1892090fa79a commit) with kernel bootargs
"slub_debug=FZPUA", I got a kernel panic in ath10k module:

[   16.058676] Unable to handle kernel paging request at virtual
address 006b6b6b6b6b6b6b
[   16.066613] Mem abort info:
[   16.069419]   ESR = 0x96000004
[   16.072481]   Exception class = DABT (current EL), IL = 32 bits
[   16.078406]   SET = 0, FnV = 0
[   16.081476]   EA = 0, S1PTW = 0
[   16.084624] Data abort info:
[   16.087513]   ISV = 0, ISS = 0x00000004
[   16.091369]   CM = 0, WnR = 0
[   16.094354] [006b6b6b6b6b6b6b] address between user and kernel address ranges
[   16.101503] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[   16.107071] Modules linked in: ath10k_sdio ath10k_core ath mac80211
cfg80211 lzo_rle lzo_compress zram asix usbnet mii joydev
[   16.118380] Process kworker/u16:2 (pid: 142, stack limit =
0x00000000082e3c57)
[   16.125597] CPU: 7 PID: 142 Comm: kworker/u16:2 Not tainted 4.19.102 #48
[   16.132287] Hardware name: MediaTek krane sku176 board (DT)
[   16.137862] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work
[ath10k_sdio]
[   16.145251] pstate: 60000005 (nZCv daif -PAN -UAO)
[   16.150051] pc : ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core]
[   16.156411] lr : ath10k_htt_htc_tx_complete+0xdc/0x1a4 [ath10k_core]
[   16.162755] sp : ffffff800888bc80
[   16.166061] x29: ffffff800888bc90 x28: ffffffd892b08c20
[   16.171363] x27: ffffffd892b173f8 x26: ffffffd892b08c20
[   16.176666] x25: ffffffd897337240 x24: ffffffd892b16b48
[   16.181968] x23: 6b6b6b6b6b6b6b6b x22: ffffff970d2a1000
[   16.187270] x21: ffffff970d2a0000 x20: ffffffd897337240
[   16.192572] x19: ffffffd892b01960 x18: 0000000000000000
[   16.197873] x17: 000000000000003c x16: ffffff970edefba0
[   16.203174] x15: 0000000000000006 x14: ffff001000000600
[   16.208475] x13: 00000000000064e6 x12: 0000000000000000
[   16.213777] x11: 0000000000000000 x10: 0000000000000000
[   16.219079] x9 : b307f4e257a4e000 x8 : b307f4e257a4e000
[   16.224391] x7 : 0000000000000000 x6 : ffffff970f970e9c
[   16.229712] x5 : 0000000000000027 x4 : 0000000000000000
[   16.235030] x3 : 000000000002ed25 x2 : ffffffd8bff94fd8
[   16.240341] x1 : ffffffd8bff8c0c8 x0 : 0000000000000034
[   16.245644] Call trace:
[   16.248109]  ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core]
[   16.254123]  ath10k_htc_notify_tx_completion+0xe4/0x118 [ath10k_core]
[   16.260559]  ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio]
[   16.266823]  process_one_work+0x208/0x408
[   16.270825]  worker_thread+0x23c/0x3e4
[   16.274566]  kthread+0x120/0x130
[   16.277788]  ret_from_fork+0x10/0x18
[   16.281357] Code: 528046a3 aa1303e0 97ffc028 f9406a97 (394002e8)
[   16.287442] ---[ end trace 3bae4173512bf484 ]---
[   16.298803] Kernel panic - not syncing: Fatal exception
[   16.304033] SMP: stopping secondary CPUs
[   16.308072] Kernel Offset: 0x1706400000 from 0xffffff8008000000
[   16.313983] CPU features: 0x0,2188200c
[   16.317721] Memory Limit: none

So it seems that the skb is used-after-free in ath10k_htt_htc_tx_complete here.

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Wen Gong]

On 2020-02-11 19:11, Pi-Hsun Shih wrote:
> On Tue, Feb 11, 2020 at 5:46 PM Wen Gong <wgong@codeaurora.org> wrote:
>> 
>> On 2020-02-11 15:03, Pi-Hsun Shih wrote:
>> > Hi,
>> >
>> > On 11/28/19 6:30 PM, Wen Gong wrote:
>> >> ...
>> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c
>> >> b/drivers/net/wireless/ath/ath10k/htt_tx.c
>> >> index a182c0944cc7..c6c4b2a4d20f 100644
>> >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
>> >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
>> >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
>> >>     void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff
>> >> *skb)
>> >>   {
>> >> +    struct ath10k_htt *htt = &ar->htt;
>> >> +    struct htt_tx_done tx_done = {0};
>> >> +    struct htt_cmd_hdr *htt_hdr;
>> >> +    struct htt_data_tx_desc *desc_hdr;
>> >> +    u16 flags1;
>> >> +
>> >>      dev_kfree_skb_any(skb);
>> >> +
>> >> +    if (!htt->disable_tx_comp)
>> >> +            return;
>> >> +
>> >> +    htt_hdr = (struct htt_cmd_hdr *)skb->data;
>> >
>> > skb is already freed on the above line (dev_kfree_skb_any) but is
>> > still used here, should the dev_kfree_skb_any be moved to the end of
>> > this function?
>> >
>> skb will not freed on the above line, please see this patch
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a
> 
> IIUC the commit only makes the skb not freed in ieee80211_tx_status,
> but it's still freed in ath10k_htt_htc_tx_complete (by
> dev_kfree_skb_any)?
> 
> While booting with this patch (and the
> 30382dd1cf3a141bfaa568ee183c1892090fa79a commit) with kernel bootargs
> "slub_debug=FZPUA", I got a kernel panic in ath10k module:
> 
> [   16.058676] Unable to handle kernel paging request at virtual
> address 006b6b6b6b6b6b6b
> [   16.066613] Mem abort info:
> [   16.069419]   ESR = 0x96000004
> [   16.072481]   Exception class = DABT (current EL), IL = 32 bits
> [   16.078406]   SET = 0, FnV = 0
> [   16.081476]   EA = 0, S1PTW = 0
> [   16.084624] Data abort info:
> [   16.087513]   ISV = 0, ISS = 0x00000004
> [   16.091369]   CM = 0, WnR = 0
> [   16.094354] [006b6b6b6b6b6b6b] address between user and kernel 
> address ranges
> [   16.101503] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [   16.107071] Modules linked in: ath10k_sdio ath10k_core ath mac80211
> cfg80211 lzo_rle lzo_compress zram asix usbnet mii joydev
> [   16.118380] Process kworker/u16:2 (pid: 142, stack limit =
> 0x00000000082e3c57)
> [   16.125597] CPU: 7 PID: 142 Comm: kworker/u16:2 Not tainted 4.19.102 
> #48
> [   16.132287] Hardware name: MediaTek krane sku176 board (DT)
> [   16.137862] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work
> [ath10k_sdio]
> [   16.145251] pstate: 60000005 (nZCv daif -PAN -UAO)
> [   16.150051] pc : ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core]
> [   16.156411] lr : ath10k_htt_htc_tx_complete+0xdc/0x1a4 [ath10k_core]
> [   16.162755] sp : ffffff800888bc80
> [   16.166061] x29: ffffff800888bc90 x28: ffffffd892b08c20
> [   16.171363] x27: ffffffd892b173f8 x26: ffffffd892b08c20
> [   16.176666] x25: ffffffd897337240 x24: ffffffd892b16b48
> [   16.181968] x23: 6b6b6b6b6b6b6b6b x22: ffffff970d2a1000
> [   16.187270] x21: ffffff970d2a0000 x20: ffffffd897337240
> [   16.192572] x19: ffffffd892b01960 x18: 0000000000000000
> [   16.197873] x17: 000000000000003c x16: ffffff970edefba0
> [   16.203174] x15: 0000000000000006 x14: ffff001000000600
> [   16.208475] x13: 00000000000064e6 x12: 0000000000000000
> [   16.213777] x11: 0000000000000000 x10: 0000000000000000
> [   16.219079] x9 : b307f4e257a4e000 x8 : b307f4e257a4e000
> [   16.224391] x7 : 0000000000000000 x6 : ffffff970f970e9c
> [   16.229712] x5 : 0000000000000027 x4 : 0000000000000000
> [   16.235030] x3 : 000000000002ed25 x2 : ffffffd8bff94fd8
> [   16.240341] x1 : ffffffd8bff8c0c8 x0 : 0000000000000034
> [   16.245644] Call trace:
> [   16.248109]  ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core]
> [   16.254123]  ath10k_htc_notify_tx_completion+0xe4/0x118 
> [ath10k_core]
> [   16.260559]  ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio]
> [   16.266823]  process_one_work+0x208/0x408
> [   16.270825]  worker_thread+0x23c/0x3e4
> [   16.274566]  kthread+0x120/0x130
> [   16.277788]  ret_from_fork+0x10/0x18
> [   16.281357] Code: 528046a3 aa1303e0 97ffc028 f9406a97 (394002e8)
> [   16.287442] ---[ end trace 3bae4173512bf484 ]---
> [   16.298803] Kernel panic - not syncing: Fatal exception
> [   16.304033] SMP: stopping secondary CPUs
> [   16.308072] Kernel Offset: 0x1706400000 from 0xffffff8008000000
> [   16.313983] CPU features: 0x0,2188200c
> [   16.317721] Memory Limit: none
> 
> So it seems that the skb is used-after-free in 
> ath10k_htt_htc_tx_complete here.
I guess the panic is because of a tx mgmt frame.
did you connect to an AP?
is the panic easy to happen?

Could you apply this change and collect message if panic(
also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable 
TX complete indication of htt for sdio)?
it will print some useful info with ath10k_warn.

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c 
b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 51f060a00b95..b1f768271331 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -555,6 +555,21 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar, 
struct sk_buff *skb)
         struct htt_data_tx_desc *desc_hdr;
         u16 flags1;

+       htt_hdr = (struct htt_cmd_hdr *)skb->data;
+
+       ath10k_warn(ar, "msg_type: %d\n", htt_hdr->msg_type);
+
+       if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) {
+               desc_hdr = (struct htt_data_tx_desc *)
+                       (skb->data + sizeof(*htt_hdr));
+               flags1 = __le16_to_cpu(desc_hdr->flags1);
+
+               if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE)
+                       ath10k_warn(ar, "htt tx mgmt\n");
+               else
+                       ath10k_warn(ar, "htt tx data\n");
+       }
+
         dev_kfree_skb_any(skb);

         if (!htt->disable_tx_comp)

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Pi-Hsun Shih]

On Wed, Feb 12, 2020 at 12:58 PM Wen Gong <wgong@codeaurora.org> wrote:
>
> On 2020-02-11 19:11, Pi-Hsun Shih wrote:
> > On Tue, Feb 11, 2020 at 5:46 PM Wen Gong <wgong@codeaurora.org> wrote:
> >>
> >> On 2020-02-11 15:03, Pi-Hsun Shih wrote:
> >> > Hi,
> >> >
> >> > On 11/28/19 6:30 PM, Wen Gong wrote:
> >> >> ...
> >> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> >> b/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> >> index a182c0944cc7..c6c4b2a4d20f 100644
> >> >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> >> >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
> >> >>     void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff
> >> >> *skb)
> >> >>   {
> >> >> +    struct ath10k_htt *htt = &ar->htt;
> >> >> +    struct htt_tx_done tx_done = {0};
> >> >> +    struct htt_cmd_hdr *htt_hdr;
> >> >> +    struct htt_data_tx_desc *desc_hdr;
> >> >> +    u16 flags1;
> >> >> +
> >> >>      dev_kfree_skb_any(skb);
> >> >> +
> >> >> +    if (!htt->disable_tx_comp)
> >> >> +            return;
> >> >> +
> >> >> +    htt_hdr = (struct htt_cmd_hdr *)skb->data;
> >> >
> >> > skb is already freed on the above line (dev_kfree_skb_any) but is
> >> > still used here, should the dev_kfree_skb_any be moved to the end of
> >> > this function?
> >> >
> >> skb will not freed on the above line, please see this patch
> >> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a
> >
> > IIUC the commit only makes the skb not freed in ieee80211_tx_status,
> > but it's still freed in ath10k_htt_htc_tx_complete (by
> > dev_kfree_skb_any)?
> >
> > While booting with this patch (and the
> > 30382dd1cf3a141bfaa568ee183c1892090fa79a commit) with kernel bootargs
> > "slub_debug=FZPUA", I got a kernel panic in ath10k module:
> >
> > [   16.058676] Unable to handle kernel paging request at virtual
> > address 006b6b6b6b6b6b6b
> > [   16.066613] Mem abort info:
> > [   16.069419]   ESR = 0x96000004
> > [   16.072481]   Exception class = DABT (current EL), IL = 32 bits
> > [   16.078406]   SET = 0, FnV = 0
> > [   16.081476]   EA = 0, S1PTW = 0
> > [   16.084624] Data abort info:
> > [   16.087513]   ISV = 0, ISS = 0x00000004
> > [   16.091369]   CM = 0, WnR = 0
> > [   16.094354] [006b6b6b6b6b6b6b] address between user and kernel
> > address ranges
> > [   16.101503] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> > [   16.107071] Modules linked in: ath10k_sdio ath10k_core ath mac80211
> > cfg80211 lzo_rle lzo_compress zram asix usbnet mii joydev
> > [   16.118380] Process kworker/u16:2 (pid: 142, stack limit =
> > 0x00000000082e3c57)
> > [   16.125597] CPU: 7 PID: 142 Comm: kworker/u16:2 Not tainted 4.19.102
> > #48
> > [   16.132287] Hardware name: MediaTek krane sku176 board (DT)
> > [   16.137862] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work
> > [ath10k_sdio]
> > [   16.145251] pstate: 60000005 (nZCv daif -PAN -UAO)
> > [   16.150051] pc : ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core]
> > [   16.156411] lr : ath10k_htt_htc_tx_complete+0xdc/0x1a4 [ath10k_core]
> > [   16.162755] sp : ffffff800888bc80
> > [   16.166061] x29: ffffff800888bc90 x28: ffffffd892b08c20
> > [   16.171363] x27: ffffffd892b173f8 x26: ffffffd892b08c20
> > [   16.176666] x25: ffffffd897337240 x24: ffffffd892b16b48
> > [   16.181968] x23: 6b6b6b6b6b6b6b6b x22: ffffff970d2a1000
> > [   16.187270] x21: ffffff970d2a0000 x20: ffffffd897337240
> > [   16.192572] x19: ffffffd892b01960 x18: 0000000000000000
> > [   16.197873] x17: 000000000000003c x16: ffffff970edefba0
> > [   16.203174] x15: 0000000000000006 x14: ffff001000000600
> > [   16.208475] x13: 00000000000064e6 x12: 0000000000000000
> > [   16.213777] x11: 0000000000000000 x10: 0000000000000000
> > [   16.219079] x9 : b307f4e257a4e000 x8 : b307f4e257a4e000
> > [   16.224391] x7 : 0000000000000000 x6 : ffffff970f970e9c
> > [   16.229712] x5 : 0000000000000027 x4 : 0000000000000000
> > [   16.235030] x3 : 000000000002ed25 x2 : ffffffd8bff94fd8
> > [   16.240341] x1 : ffffffd8bff8c0c8 x0 : 0000000000000034
> > [   16.245644] Call trace:
> > [   16.248109]  ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core]
> > [   16.254123]  ath10k_htc_notify_tx_completion+0xe4/0x118
> > [ath10k_core]
> > [   16.260559]  ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio]
> > [   16.266823]  process_one_work+0x208/0x408
> > [   16.270825]  worker_thread+0x23c/0x3e4
> > [   16.274566]  kthread+0x120/0x130
> > [   16.277788]  ret_from_fork+0x10/0x18
> > [   16.281357] Code: 528046a3 aa1303e0 97ffc028 f9406a97 (394002e8)
> > [   16.287442] ---[ end trace 3bae4173512bf484 ]---
> > [   16.298803] Kernel panic - not syncing: Fatal exception
> > [   16.304033] SMP: stopping secondary CPUs
> > [   16.308072] Kernel Offset: 0x1706400000 from 0xffffff8008000000
> > [   16.313983] CPU features: 0x0,2188200c
> > [   16.317721] Memory Limit: none
> >
> > So it seems that the skb is used-after-free in
> > ath10k_htt_htc_tx_complete here.
> I guess the panic is because of a tx mgmt frame.
> did you connect to an AP?
No, I disabled wifi on UI before adding the kernel bootargs, so it
should not be connected to any AP.
> is the panic easy to happen?
Yes, this happens on every boot (early in boot before entering UI)
with kernel bootargs "slub_debug=FZPUA" added.
>
> Could you apply this change and collect message if panic(
> also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable
> TX complete indication of htt for sdio)?
> it will print some useful info with ath10k_warn.
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c
> b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index 51f060a00b95..b1f768271331 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -555,6 +555,21 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar,
> struct sk_buff *skb)
>          struct htt_data_tx_desc *desc_hdr;
>          u16 flags1;
>
> +       htt_hdr = (struct htt_cmd_hdr *)skb->data;
> +
> +       ath10k_warn(ar, "msg_type: %d\n", htt_hdr->msg_type);
> +
> +       if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) {
> +               desc_hdr = (struct htt_data_tx_desc *)
> +                       (skb->data + sizeof(*htt_hdr));
> +               flags1 = __le16_to_cpu(desc_hdr->flags1);
> +
> +               if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE)
> +                       ath10k_warn(ar, "htt tx mgmt\n");
> +               else
> +                       ath10k_warn(ar, "htt tx data\n");
> +       }
> +
>          dev_kfree_skb_any(skb);
>
>          if (!htt->disable_tx_comp)
Output as follows:

[   10.747482] ath10k_sdio mmc1:0001:1: msg_type: 0
[   10.749295] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3
cal otp max-sta 32 raw 0 hwcrypto 1
[   10.752243] Unable to handle kernel paging request at virtual
address 006b6b6b6b6b6b6b
[   10.769674] Mem abort info:
[   10.772514]   ESR = 0x96000004
[   10.775625]   Exception class = DABT (current EL), IL = 32 bits
[   10.781609]   SET = 0, FnV = 0
[   10.784699]   EA = 0, S1PTW = 0
[   10.787889] Data abort info:
[   10.790839]   ISV = 0, ISS = 0x00000004
[   10.794711]   CM = 0, WnR = 0
[   10.797714] [006b6b6b6b6b6b6b] address between user and kernel address ranges
[   10.804911] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[   10.810488] Modules linked in: asix usbnet mii ath10k_sdio
ath10k_core ath lzo_rle mac80211 lzo_compress zram cfg80211 joydev
[   10.821800] Process kworker/u16:1 (pid: 140, stack limit =
0x0000000008a1ed57)
[   10.829017] CPU: 4 PID: 140 Comm: kworker/u16:1 Tainted: G        W
        4.19.102 #49
[   10.837097] Hardware name: MediaTek krane sku176 board (DT)
[   10.842670] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work
[ath10k_sdio]
[   10.850059] pstate: 60000005 (nZCv daif -PAN -UAO)
[   10.854860] pc : ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core]
[   10.861225] lr : ath10k_htt_htc_tx_complete+0x8c/0x134 [ath10k_core]
[   10.867568] sp : ffffff800888bc90
[   10.870874] x29: ffffff800888bca0 x28: fffffffd69548be8
[   10.876177] x27: fffffffd695573f8 x26: fffffffd69548be8
[   10.881480] x25: fffffffd742f1e40 x24: fffffffd69556b48
[   10.886782] x23: fffffffd69556b10 x22: fffffffd695c7e80
[   10.892084] x21: 6b6b6b6b6b6b6b6b x20: fffffffd742f1e40
[   10.897386] x19: fffffffd69541960 x18: 0000000000000000
[   10.902696] x17: 000000000000003c x16: ffffffa964a7d36c
[   10.908004] x15: fffffffd742f3e80 x14: 0000000000000280
[   10.913306] x13: 0000000000000001 x12: 0000000000000000
[   10.918607] x11: 0000000000000000 x10: 0000000000000000
[   10.923908] x9 : 2edc72d89d761200 x8 : 0000000000000001
[   10.929209] x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020
[   10.934511] x5 : 000000000000005a x4 : 0000000000000000
[   10.939812] x3 : 0000000000000010 x2 : 0000000000000008
[   10.945122] x1 : 0000000000000000 x0 : 0000000000000000
[   10.950439] Call trace:
[   10.952904]  ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core]
[   10.958918]  ath10k_htc_notify_tx_completion+0xe4/0x118 [ath10k_core]
[   10.965366]  ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio]
[   10.971637]  process_one_work+0x208/0x408
[   10.975638]  worker_thread+0x23c/0x3e4
[   10.979379]  kthread+0x120/0x130
[   10.982602]  ret_from_fork+0x10/0x18
[   10.986178] Code: 52820f08 38686a68 340003a8 f9406a95 (394002a8)
[   10.992266] ---[ end trace 3ed9b11cd8f60113 ]---
[   11.004351] Kernel panic - not syncing: Fatal exception
[   11.009586] SMP: stopping secondary CPUs
[   11.013519] Kernel Offset: 0x295c200000 from 0xffffff8008000000
[   11.019433] CPU features: 0x0,2188200c
[   11.023172] Memory Limit: none

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Wen Gong]

On 2020-02-12 13:31, Pi-Hsun Shih wrote:

> Output as follows:
> 
> [   10.747482] ath10k_sdio mmc1:0001:1: msg_type: 0
> [   10.749295] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3
> cal otp max-sta 32 raw 0 hwcrypto 1
> [   10.752243] Unable to handle kernel paging request at virtual
> address 006b6b6b6b6b6b6b
> [   10.769674] Mem abort info:
> [   10.772514]   ESR = 0x96000004
> [   10.775625]   Exception class = DABT (current EL), IL = 32 bits
> [   10.781609]   SET = 0, FnV = 0
> [   10.784699]   EA = 0, S1PTW = 0
> [   10.787889] Data abort info:
> [   10.790839]   ISV = 0, ISS = 0x00000004
> [   10.794711]   CM = 0, WnR = 0
> [   10.797714] [006b6b6b6b6b6b6b] address between user and kernel 
> address ranges
> [   10.804911] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [   10.810488] Modules linked in: asix usbnet mii ath10k_sdio
> ath10k_core ath lzo_rle mac80211 lzo_compress zram cfg80211 joydev
> [   10.821800] Process kworker/u16:1 (pid: 140, stack limit =
> 0x0000000008a1ed57)
> [   10.829017] CPU: 4 PID: 140 Comm: kworker/u16:1 Tainted: G        W
>         4.19.102 #49
> [   10.837097] Hardware name: MediaTek krane sku176 board (DT)
> [   10.842670] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work
> [ath10k_sdio]
> [   10.850059] pstate: 60000005 (nZCv daif -PAN -UAO)
> [   10.854860] pc : ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core]
> [   10.861225] lr : ath10k_htt_htc_tx_complete+0x8c/0x134 [ath10k_core]
> [   10.867568] sp : ffffff800888bc90
> [   10.870874] x29: ffffff800888bca0 x28: fffffffd69548be8
> [   10.876177] x27: fffffffd695573f8 x26: fffffffd69548be8
> [   10.881480] x25: fffffffd742f1e40 x24: fffffffd69556b48
> [   10.886782] x23: fffffffd69556b10 x22: fffffffd695c7e80
> [   10.892084] x21: 6b6b6b6b6b6b6b6b x20: fffffffd742f1e40
> [   10.897386] x19: fffffffd69541960 x18: 0000000000000000
> [   10.902696] x17: 000000000000003c x16: ffffffa964a7d36c
> [   10.908004] x15: fffffffd742f3e80 x14: 0000000000000280
> [   10.913306] x13: 0000000000000001 x12: 0000000000000000
> [   10.918607] x11: 0000000000000000 x10: 0000000000000000
> [   10.923908] x9 : 2edc72d89d761200 x8 : 0000000000000001
> [   10.929209] x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020
> [   10.934511] x5 : 000000000000005a x4 : 0000000000000000
> [   10.939812] x3 : 0000000000000010 x2 : 0000000000000008
> [   10.945122] x1 : 0000000000000000 x0 : 0000000000000000
> [   10.950439] Call trace:
> [   10.952904]  ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core]
> [   10.958918]  ath10k_htc_notify_tx_completion+0xe4/0x118 
> [ath10k_core]
> [   10.965366]  ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio]
> [   10.971637]  process_one_work+0x208/0x408
> [   10.975638]  worker_thread+0x23c/0x3e4
> [   10.979379]  kthread+0x120/0x130
> [   10.982602]  ret_from_fork+0x10/0x18
> [   10.986178] Code: 52820f08 38686a68 340003a8 f9406a95 (394002a8)
> [   10.992266] ---[ end trace 3ed9b11cd8f60113 ]---
> [   11.004351] Kernel panic - not syncing: Fatal exception
> [   11.009586] SMP: stopping secondary CPUs
> [   11.013519] Kernel Offset: 0x295c200000 from 0xffffff8008000000
> [   11.019433] CPU features: 0x0,2188200c
> [   11.023172] Memory Limit: none
Thanks.
I have made change again to fix the panic.
could you try again with this change?
(also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable 
TX complete indication of htt for sdio)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c 
b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 51f060a00b95..7bfdeb1298a5 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -554,20 +554,30 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar, 
struct sk_buff *skb)
         struct htt_cmd_hdr *htt_hdr;
         struct htt_data_tx_desc *desc_hdr;
         u16 flags1;
+       u8 msg_type;
+
+       if (htt->disable_tx_comp) {
+               htt_hdr = (struct htt_cmd_hdr *)skb->data;
+               msg_type = htt_hdr->msg_type;
+               ath10k_warn(ar, "msg_type: %d\n", msg_type);
+
+               if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) {
+                       desc_hdr = (struct htt_data_tx_desc *)
+                               (skb->data + sizeof(*htt_hdr));
+                       flags1 = __le16_to_cpu(desc_hdr->flags1);
+
+                       if (flags1 & 
HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE)
+                               ath10k_warn(ar, "htt tx mgmt\n");
+                       else
+                               ath10k_warn(ar, "htt tx data\n");
+               }
+       }

         dev_kfree_skb_any(skb);

-       if (!htt->disable_tx_comp)
+       if ((!htt->disable_tx_comp) || (msg_type != 
HTT_H2T_MSG_TYPE_TX_FRM))
                 return;

-       htt_hdr = (struct htt_cmd_hdr *)skb->data;
-       if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM)
-               return;
-
-       desc_hdr = (struct htt_data_tx_desc *)
-               (skb->data + sizeof(*htt_hdr));
-       flags1 = __le16_to_cpu(desc_hdr->flags1);
-
         ath10k_dbg(ar, ATH10K_DBG_HTT,
                    "htt tx complete msdu id:%u ,flags1:%x\n",
                    __le16_to_cpu(desc_hdr->id), flags1);

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Pi-Hsun Shih]

Tested that the patch fix the kernel panic, thanks.

For the fixed version:
Tested-by: Pi-Hsun Shih <pihsun@chromium.org>




On Wed, Feb 12, 2020 at 2:47 PM Wen Gong <wgong@codeaurora.org> wrote:
>
> On 2020-02-12 13:31, Pi-Hsun Shih wrote:
>
> > Output as follows:
> >
> > [   10.747482] ath10k_sdio mmc1:0001:1: msg_type: 0
> > [   10.749295] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3
> > cal otp max-sta 32 raw 0 hwcrypto 1
> > [   10.752243] Unable to handle kernel paging request at virtual
> > address 006b6b6b6b6b6b6b
> > [   10.769674] Mem abort info:
> > [   10.772514]   ESR = 0x96000004
> > [   10.775625]   Exception class = DABT (current EL), IL = 32 bits
> > [   10.781609]   SET = 0, FnV = 0
> > [   10.784699]   EA = 0, S1PTW = 0
> > [   10.787889] Data abort info:
> > [   10.790839]   ISV = 0, ISS = 0x00000004
> > [   10.794711]   CM = 0, WnR = 0
> > [   10.797714] [006b6b6b6b6b6b6b] address between user and kernel
> > address ranges
> > [   10.804911] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> > [   10.810488] Modules linked in: asix usbnet mii ath10k_sdio
> > ath10k_core ath lzo_rle mac80211 lzo_compress zram cfg80211 joydev
> > [   10.821800] Process kworker/u16:1 (pid: 140, stack limit =
> > 0x0000000008a1ed57)
> > [   10.829017] CPU: 4 PID: 140 Comm: kworker/u16:1 Tainted: G        W
> >         4.19.102 #49
> > [   10.837097] Hardware name: MediaTek krane sku176 board (DT)
> > [   10.842670] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work
> > [ath10k_sdio]
> > [   10.850059] pstate: 60000005 (nZCv daif -PAN -UAO)
> > [   10.854860] pc : ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core]
> > [   10.861225] lr : ath10k_htt_htc_tx_complete+0x8c/0x134 [ath10k_core]
> > [   10.867568] sp : ffffff800888bc90
> > [   10.870874] x29: ffffff800888bca0 x28: fffffffd69548be8
> > [   10.876177] x27: fffffffd695573f8 x26: fffffffd69548be8
> > [   10.881480] x25: fffffffd742f1e40 x24: fffffffd69556b48
> > [   10.886782] x23: fffffffd69556b10 x22: fffffffd695c7e80
> > [   10.892084] x21: 6b6b6b6b6b6b6b6b x20: fffffffd742f1e40
> > [   10.897386] x19: fffffffd69541960 x18: 0000000000000000
> > [   10.902696] x17: 000000000000003c x16: ffffffa964a7d36c
> > [   10.908004] x15: fffffffd742f3e80 x14: 0000000000000280
> > [   10.913306] x13: 0000000000000001 x12: 0000000000000000
> > [   10.918607] x11: 0000000000000000 x10: 0000000000000000
> > [   10.923908] x9 : 2edc72d89d761200 x8 : 0000000000000001
> > [   10.929209] x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020
> > [   10.934511] x5 : 000000000000005a x4 : 0000000000000000
> > [   10.939812] x3 : 0000000000000010 x2 : 0000000000000008
> > [   10.945122] x1 : 0000000000000000 x0 : 0000000000000000
> > [   10.950439] Call trace:
> > [   10.952904]  ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core]
> > [   10.958918]  ath10k_htc_notify_tx_completion+0xe4/0x118
> > [ath10k_core]
> > [   10.965366]  ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio]
> > [   10.971637]  process_one_work+0x208/0x408
> > [   10.975638]  worker_thread+0x23c/0x3e4
> > [   10.979379]  kthread+0x120/0x130
> > [   10.982602]  ret_from_fork+0x10/0x18
> > [   10.986178] Code: 52820f08 38686a68 340003a8 f9406a95 (394002a8)
> > [   10.992266] ---[ end trace 3ed9b11cd8f60113 ]---
> > [   11.004351] Kernel panic - not syncing: Fatal exception
> > [   11.009586] SMP: stopping secondary CPUs
> > [   11.013519] Kernel Offset: 0x295c200000 from 0xffffff8008000000
> > [   11.019433] CPU features: 0x0,2188200c
> > [   11.023172] Memory Limit: none
> Thanks.
> I have made change again to fix the panic.
> could you try again with this change?
> (also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable
> TX complete indication of htt for sdio)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c
> b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index 51f060a00b95..7bfdeb1298a5 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -554,20 +554,30 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar,
> struct sk_buff *skb)
>          struct htt_cmd_hdr *htt_hdr;
>          struct htt_data_tx_desc *desc_hdr;
>          u16 flags1;
> +       u8 msg_type;
> +
> +       if (htt->disable_tx_comp) {
> +               htt_hdr = (struct htt_cmd_hdr *)skb->data;
> +               msg_type = htt_hdr->msg_type;
> +               ath10k_warn(ar, "msg_type: %d\n", msg_type);
> +
> +               if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) {
> +                       desc_hdr = (struct htt_data_tx_desc *)
> +                               (skb->data + sizeof(*htt_hdr));
> +                       flags1 = __le16_to_cpu(desc_hdr->flags1);
> +
> +                       if (flags1 &
> HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE)
> +                               ath10k_warn(ar, "htt tx mgmt\n");
> +                       else
> +                               ath10k_warn(ar, "htt tx data\n");
> +               }
> +       }
>
>          dev_kfree_skb_any(skb);
>
> -       if (!htt->disable_tx_comp)
> +       if ((!htt->disable_tx_comp) || (msg_type !=
> HTT_H2T_MSG_TYPE_TX_FRM))
>                  return;
>
> -       htt_hdr = (struct htt_cmd_hdr *)skb->data;
> -       if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM)
> -               return;
> -
> -       desc_hdr = (struct htt_data_tx_desc *)
> -               (skb->data + sizeof(*htt_hdr));
> -       flags1 = __le16_to_cpu(desc_hdr->flags1);
> -
>          ath10k_dbg(ar, ATH10K_DBG_HTT,
>                     "htt tx complete msdu id:%u ,flags1:%x\n",
>                     __le16_to_cpu(desc_hdr->id), flags1);

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Wen Gong]

On 2020-02-12 15:08, Pi-Hsun Shih wrote:
> Tested that the patch fix the kernel panic, thanks.
> 
> For the fixed version:
> Tested-by: Pi-Hsun Shih <pihsun@chromium.org>
> 
Thanks Pi-Hsun's quick test,
could you also give me the message log:
dmesg | grep ath

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Pi-Hsun Shih]

On Wed, Feb 12, 2020 at 3:31 PM Wen Gong <wgong@codeaurora.org> wrote:
>
> On 2020-02-12 15:08, Pi-Hsun Shih wrote:
> > Tested that the patch fix the kernel panic, thanks.
> >
> > For the fixed version:
> > Tested-by: Pi-Hsun Shih <pihsun@chromium.org>
> >
> Thanks Pi-Hsun's quick test,
> could you also give me the message log:
> dmesg | grep ath

[   11.462398] ath10k_sdio mmc1:0001:1: qca6174 hw3.2 sdio target
0x05030000 chip_id 0x00000000 sub 0000:0000
[   11.472131] ath10k_sdio mmc1:0001:1: kconfig debug 1 debugfs 1
tracing 1 dfs 0 testmode 1
[   11.481490] ath10k_sdio mmc1:0001:1: firmware ver
WLAN.RMH.4.4.1-00042 api 6 features wowlan,ignore-otp crc32 ac2d4918
[   11.667020] ath10k_sdio mmc1:0001:1: board_file api 2 bmi_id 0:4
crc32 e74847dc
[   12.035056] ath10k_sdio mmc1:0001:1: msg_type: 0
[   12.036919] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3
cal otp max-sta 32 raw 0 hwcrypto 1
[   12.039878] ath10k_sdio mmc1:0001:1: msg_type: 2
[   12.053816] ath10k_sdio mmc1:0001:1: msg_type: 5
[   12.235403] ath: EEPROM regdomain: 0x6c
[   12.239895] ath: EEPROM indicates we should expect a direct regpair map
[   12.247223] ath: Country alpha2 being used: 00
[   12.251973] ath: Regpair used: 0x6c

Re: [PATCH v8 1/4] ath10k: disable TX complete indication of htt for sdio

[Wen Gong]

On 2020-02-12 15:45, Pi-Hsun Shih wrote:
> On Wed, Feb 12, 2020 at 3:31 PM Wen Gong <wgong@codeaurora.org> wrote:
>> 
>> On 2020-02-12 15:08, Pi-Hsun Shih wrote:
>> > Tested that the patch fix the kernel panic, thanks.
>> >
>> > For the fixed version:
>> > Tested-by: Pi-Hsun Shih <pihsun@chromium.org>
>> >
>> Thanks Pi-Hsun's quick test,
>> could you also give me the message log:
>> dmesg | grep ath
> 
> [   11.462398] ath10k_sdio mmc1:0001:1: qca6174 hw3.2 sdio target
> 0x05030000 chip_id 0x00000000 sub 0000:0000
> [   11.472131] ath10k_sdio mmc1:0001:1: kconfig debug 1 debugfs 1
> tracing 1 dfs 0 testmode 1
> [   11.481490] ath10k_sdio mmc1:0001:1: firmware ver
> WLAN.RMH.4.4.1-00042 api 6 features wowlan,ignore-otp crc32 ac2d4918
> [   11.667020] ath10k_sdio mmc1:0001:1: board_file api 2 bmi_id 0:4
> crc32 e74847dc
> [   12.035056] ath10k_sdio mmc1:0001:1: msg_type: 0
> [   12.036919] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3
> cal otp max-sta 32 raw 0 hwcrypto 1
> [   12.039878] ath10k_sdio mmc1:0001:1: msg_type: 2
> [   12.053816] ath10k_sdio mmc1:0001:1: msg_type: 5
> [   12.235403] ath: EEPROM regdomain: 0x6c
> [   12.239895] ath: EEPROM indicates we should expect a direct regpair 
> map
> [   12.247223] ath: Country alpha2 being used: 00
> [   12.251973] ath: Regpair used: 0x6c
Thanks.
new patch sent:
https://patchwork.kernel.org/patch/11377827/