[PATCH 00/11] SDIO support for ath10k

[PATCH 00/11] SDIO support for ath10k

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

This patchset, generated against master-pending branch, enables a fully
functional SDIO interface driver for ath10k.  Patches have been verified on
QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
and P2P modes.

The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
with the board data from respective SDIO card vendors. Receive performance
matches the QCA reference driver when used with SDIO3.0 enabled platforms.
iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s

This patchset differs from the previous high latency patches, specific to SDIO.
HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
this flag, the management frames are not sent out by the firmware. Possibility
of management frames being sent via WMI and data frames through the reduced Tx
completion needs to be probed further.

Further improvements can be done on the transmit path by implementing packet
bundle. Scatter Gather is another area of improvement for both Transmit and
Receive, but may not work on all platforms

Known issues: Surprise removal of the card, when the device is in connected
state, delays sdio function remove due to delayed WMI command failures.
Existing ath10k framework can not differentiate between a kernel module
removal and the surprise removal of teh card.

Alagu Sankar (11):
  ath10k_sdio: sdio htt data transfer fixes
  ath10k_sdio: wb396 reference card fix
  ath10k_sdio: DMA bounce buffers for read write
  ath10k_sdio: reduce transmit msdu count
  ath10k_sdio: use clean packet headers
  ath10k_sdio: high latency fixes for beacon buffer
  ath10k_sdio: fix rssi indication
  ath10k_sdio: common read write
  ath10k_sdio: virtual scatter gather for receive
  ath10k_sdio: enable firmware crash dump
  ath10k_sdio: hif start once addition

 drivers/net/wireless/ath/ath10k/core.c    |  35 ++-
 drivers/net/wireless/ath/ath10k/debug.c   |   3 +
 drivers/net/wireless/ath/ath10k/htc.c     |   4 +-
 drivers/net/wireless/ath/ath10k/htc.h     |   1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c  |  19 +-
 drivers/net/wireless/ath/ath10k/htt_tx.c  |  24 +-
 drivers/net/wireless/ath/ath10k/hw.c      |   2 +
 drivers/net/wireless/ath/ath10k/hw.h      |   1 +
 drivers/net/wireless/ath/ath10k/mac.c     |  31 ++-
 drivers/net/wireless/ath/ath10k/sdio.c    | 398 ++++++++++++++++++++++--------
 drivers/net/wireless/ath/ath10k/sdio.h    |  10 +-
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +-
 12 files changed, 403 insertions(+), 127 deletions(-)

-- 
1.9.1

[PATCH 01/11] ath10k_sdio: sdio htt data transfer fixes

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

The ath10k sdio firmware does not allow transmitting packets with the
reduced tx completion HI_ACS option. sdio firmware uses 1544 as
alternate credit size, which is not big enough for the maximum sized
mac80211 frames. Disable both these HI_ACS flags for SDIO.

Transmit completion for SDIO is similar to PCIe, via the T2H message
HTT_T2H_MSG_TYPE_TX_COMPL_IND.  Modify the high latency path to allow
SDIO modules to use the htt transmit completion path. This differs from
the high latency path taken by the USB devices.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/core.c   | 20 +++++++++++++++++---
 drivers/net/wireless/ath/ath10k/htt_rx.c |  6 ++++--
 drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +++++++++++++++++++-----
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 4351341..b4f66cd 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -494,11 +494,25 @@ static void ath10k_init_sdio(struct ath10k *ar)
 	ath10k_bmi_write32(ar, hi_mbox_isr_yield_limit, 99);
 	ath10k_bmi_read32(ar, hi_acs_flags, &param);
 
-	param |= (HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_SET |
-		  HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET |
-		  HI_ACS_FLAGS_ALT_DATA_CREDIT_SIZE);
+	/* 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;
+
+	/* 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;
 
+	param |= HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_SET;
 	ath10k_bmi_write32(ar, hi_acs_flags, param);
+
+	/* Explicitly set fwlog prints to zero as target may turn it on
+	 * based on scratch registers.
+	 */
+	ath10k_bmi_read32(ar, hi_option_flag, &param);
+	param |= HI_OPTION_DISABLE_DBGLOG;
+	ath10k_bmi_write32(ar, hi_option_flag, param);
 }
 
 static int ath10k_init_configure_target(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 569edd0..f025363 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1764,7 +1764,9 @@ static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
 		 *  Note that with only one concurrent reader and one concurrent
 		 *  writer, you don't need extra locking to use these macro.
 		 */
-		if (!kfifo_put(&htt->txdone_fifo, tx_done)) {
+		if (ar->hif.bus == ATH10K_BUS_SDIO) {
+			ath10k_txrx_tx_unref(htt, &tx_done);
+		} else if (!kfifo_put(&htt->txdone_fifo, tx_done)) {
 			ath10k_warn(ar, "txdone fifo overrun, msdu_id %d status %d\n",
 				    tx_done.msdu_id, tx_done.status);
 			ath10k_txrx_tx_unref(htt, &tx_done);
@@ -2541,7 +2543,7 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		break;
 	}
 	case HTT_T2H_MSG_TYPE_TX_COMPL_IND:
-		if (!ar->is_high_latency)
+		if (!(ar->hif.bus == ATH10K_BUS_USB))
 			ath10k_htt_rx_tx_compl_ind(htt->ar, skb);
 		break;
 	case HTT_T2H_MSG_TYPE_SEC_IND: {
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index c74fc13..d7f59a2 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -153,7 +153,7 @@ void ath10k_htt_tx_txq_update(struct ieee80211_hw *hw,
 
 void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt)
 {
-	if (htt->ar->is_high_latency)
+	if (htt->ar->hif.bus == ATH10K_BUS_USB)
 		return;
 
 	lockdep_assert_held(&htt->tx_lock);
@@ -165,7 +165,7 @@ void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt)
 
 int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
 {
-	if (htt->ar->is_high_latency)
+	if (htt->ar->hif.bus == ATH10K_BUS_USB)
 		return 0;
 
 	lockdep_assert_held(&htt->tx_lock);
@@ -454,7 +454,7 @@ void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
 		return;
 
 	ath10k_htt_tx_free_cont_txbuf(htt);
-	if (!htt->ar->is_high_latency)
+	if (!(htt->ar->hif.bus == ATH10K_BUS_USB))
 		ath10k_htt_tx_free_txq(htt);
 	ath10k_htt_tx_free_cont_frag_desc(htt);
 	ath10k_htt_tx_free_txdone_fifo(htt);
@@ -475,7 +475,8 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
 
 void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
 {
-	dev_kfree_skb_any(skb);
+	if (!(ar->hif.bus == ATH10K_BUS_SDIO))
+		dev_kfree_skb_any(skb);
 }
 
 void ath10k_htt_hif_tx_complete(struct ath10k *ar, struct sk_buff *skb)
@@ -975,6 +976,7 @@ int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
 	u8 tid = ath10k_htt_tx_get_tid(msdu, is_eth);
 	u8 flags0 = 0;
 	u16 flags1 = 0;
+	u16 msdu_id = 0;
 
 	data_len = msdu->len;
 
@@ -1022,6 +1024,18 @@ int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
 		}
 	}
 
+	if (ar->hif.bus == ATH10K_BUS_SDIO) {
+		flags1 |= HTT_DATA_TX_DESC_FLAGS1_POSTPONED;
+		spin_lock_bh(&htt->tx_lock);
+		res = ath10k_htt_tx_alloc_msdu_id(htt, msdu);
+		spin_unlock_bh(&htt->tx_lock);
+		if (res < 0) {
+			ath10k_err(ar, "msdu_id allocation failed %d\n", res);
+			goto out;
+		}
+		msdu_id = res;
+	}
+
 	skb_push(msdu, sizeof(*cmd_hdr));
 	skb_push(msdu, sizeof(*tx_desc));
 	cmd_hdr = (struct htt_cmd_hdr *)msdu->data;
@@ -1031,7 +1045,7 @@ int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
 	tx_desc->flags0 = flags0;
 	tx_desc->flags1 = __cpu_to_le16(flags1);
 	tx_desc->len = __cpu_to_le16(data_len);
-	tx_desc->id = 0;
+	tx_desc->id = __cpu_to_le16(msdu_id);
 	tx_desc->frags_paddr = 0; /* always zero */
 	/* Initialize peer_id to INVALID_PEER because this is NOT
 	 * Reinjection path
-- 
1.9.1

[PATCH 02/11] ath10k_sdio: wb396 reference card fix

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

The QCA9377-3 WB396 sdio reference card does not get initialized
due to the conflict in uart gpio pins. This fix is not required
for other QCA9377 sdio cards.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index b4f66cd..86247c8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1708,8 +1708,15 @@ static int ath10k_init_uart(struct ath10k *ar)
 		return ret;
 	}
 
-	if (!uart_print)
+	if (!uart_print) {
+		/* Hack: override dbg TX pin to avoid side effects of default
+		 * GPIO_6 in QCA9377 WB396 reference card
+		 */
+		if (ar->hif.bus == ATH10K_BUS_SDIO)
+			ath10k_bmi_write32(ar, hi_dbg_uart_txpin,
+					   ar->hw_params.uart_pin);
 		return 0;
+	}
 
 	ret = ath10k_bmi_write32(ar, hi_dbg_uart_txpin, ar->hw_params.uart_pin);
 	if (ret) {
-- 
1.9.1

[PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

Some SD host controllers still need bounce buffers for SDIO data
transfers. While the transfers worked fine on x86 platforms,
this is found to be required for i.MX6 based systems.

Changes are similar to and derived from the ath6kl sdio driver.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 59 ++++++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/sdio.h |  5 +++
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 03a69e5..77d4fa4 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -34,8 +34,20 @@
 #include "trace.h"
 #include "sdio.h"
 
+#define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
+
 /* inlined helper functions */
 
+/* Macro to check if DMA buffer is WORD-aligned and DMA-able.
+ * Most host controllers assume the buffer is DMA'able and will
+ * bug-check otherwise (i.e. buffers on the stack). virt_addr_valid
+ * check fails on stack memory.
+ */
+static inline bool buf_needs_bounce(const u8 *buf)
+{
+	return ((unsigned long)buf & 0x3) || !virt_addr_valid(buf);
+}
+
 static inline int ath10k_sdio_calc_txrx_padded_len(struct ath10k_sdio *ar_sdio,
 						   size_t len)
 {
@@ -303,15 +315,29 @@ static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct sdio_func *func = ar_sdio->func;
+	bool bounced = false;
+	u8 *tbuf = NULL;
 	int ret;
 
+	if (buf_needs_bounce(buf)) {
+		if (!ar_sdio->dma_buffer)
+			return -ENOMEM;
+		mutex_lock(&ar_sdio->dma_buffer_mutex);
+		tbuf = ar_sdio->dma_buffer;
+		bounced = true;
+	} else {
+		tbuf = buf;
+	}
+
 	sdio_claim_host(func);
 
-	ret = sdio_memcpy_fromio(func, buf, addr, len);
+	ret = sdio_memcpy_fromio(func, tbuf, addr, len);
 	if (ret) {
 		ath10k_warn(ar, "failed to read from address 0x%x: %d\n",
 			    addr, ret);
 		goto out;
+	} else if (bounced) {
+		memcpy(buf, tbuf, len);
 	}
 
 	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio read addr 0x%x buf 0x%p len %zu\n",
@@ -320,6 +346,8 @@ static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
 
 out:
 	sdio_release_host(func);
+	if (bounced)
+		mutex_unlock(&ar_sdio->dma_buffer_mutex);
 
 	return ret;
 }
@@ -328,14 +356,27 @@ static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct sdio_func *func = ar_sdio->func;
+	bool bounced = false;
+	u8 *tbuf = NULL;
 	int ret;
 
+	if (buf_needs_bounce(buf)) {
+		if (!ar_sdio->dma_buffer)
+			return -ENOMEM;
+		mutex_lock(&ar_sdio->dma_buffer_mutex);
+		tbuf = ar_sdio->dma_buffer;
+		memcpy(tbuf, buf, len);
+		bounced = true;
+	} else {
+		tbuf = (u8 *)buf;
+	}
+
 	sdio_claim_host(func);
 
 	/* For some reason toio() doesn't have const for the buffer, need
 	 * an ugly hack to workaround that.
 	 */
-	ret = sdio_memcpy_toio(func, addr, (void *)buf, len);
+	ret = sdio_memcpy_toio(func, addr, (void *)tbuf, len);
 	if (ret) {
 		ath10k_warn(ar, "failed to write to address 0x%x: %d\n",
 			    addr, ret);
@@ -348,6 +389,8 @@ static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_
 
 out:
 	sdio_release_host(func);
+	if (bounced)
+		mutex_unlock(&ar_sdio->dma_buffer_mutex);
 
 	return ret;
 }
@@ -1978,6 +2021,12 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 		goto err_free_en_reg;
 	}
 
+	ar_sdio->dma_buffer = kzalloc(ATH10K_SDIO_DMA_BUF_SIZE, GFP_KERNEL);
+	if (!ar_sdio->dma_buffer) {
+		ret = -ENOMEM;
+		goto err_free_bmi_buf;
+	}
+
 	ar_sdio->func = func;
 	sdio_set_drvdata(func, ar_sdio);
 
@@ -1987,6 +2036,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 	spin_lock_init(&ar_sdio->lock);
 	spin_lock_init(&ar_sdio->wr_async_lock);
 	mutex_init(&ar_sdio->irq_data.mtx);
+	mutex_init(&ar_sdio->dma_buffer_mutex);
 
 	INIT_LIST_HEAD(&ar_sdio->bus_req_freeq);
 	INIT_LIST_HEAD(&ar_sdio->wr_asyncq);
@@ -1995,7 +2045,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 	ar_sdio->workqueue = create_singlethread_workqueue("ath10k_sdio_wq");
 	if (!ar_sdio->workqueue) {
 		ret = -ENOMEM;
-		goto err_free_bmi_buf;
+		goto err_dma;
 	}
 
 	for (i = 0; i < ATH10K_SDIO_BUS_REQUEST_MAX_NUM; i++)
@@ -2040,6 +2090,8 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 
 err_free_wq:
 	destroy_workqueue(ar_sdio->workqueue);
+err_dma:
+	kfree(ar_sdio->dma_buffer);
 err_free_bmi_buf:
 	kfree(ar_sdio->bmi_buf);
 err_free_en_reg:
@@ -2065,6 +2117,7 @@ static void ath10k_sdio_remove(struct sdio_func *func)
 	cancel_work_sync(&ar_sdio->wr_async_work);
 	ath10k_core_unregister(ar);
 	ath10k_core_destroy(ar);
+	kfree(ar_sdio->dma_buffer);
 }
 
 static const struct sdio_device_id ath10k_sdio_devices[] = {
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index 4ff7b54..718b8b7 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -207,6 +207,11 @@ struct ath10k_sdio {
 	struct ath10k *ar;
 	struct ath10k_sdio_irq_data irq_data;
 
+	u8 *dma_buffer;
+
+	/* protects access to dma_buffer */
+	struct mutex dma_buffer_mutex;
+
 	/* temporary buffer for BMI requests */
 	u8 *bmi_buf;
 
-- 
1.9.1

[PATCH 04/11] ath10k_sdio: reduce transmit msdu count

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

Reduce the transmit MSDU count for SDIO, to match with the descriptors
as used by the firmware. This also acts as a high watermark level for
transmit. Too many packets to the firmware results in transmit overflow
interrupt.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/core.c    | 6 +++++-
 drivers/net/wireless/ath/ath10k/hw.h      | 1 +
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 86247c8..9de49f5 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1968,7 +1968,11 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
 		ar->max_num_stations = TARGET_TLV_NUM_STATIONS;
 		ar->max_num_vdevs = TARGET_TLV_NUM_VDEVS;
 		ar->max_num_tdls_vdevs = TARGET_TLV_NUM_TDLS_VDEVS;
-		ar->htt.max_num_pending_tx = TARGET_TLV_NUM_MSDU_DESC;
+		if (ar->hif.bus == ATH10K_BUS_SDIO)
+			ar->htt.max_num_pending_tx =
+				TARGET_TLV_NUM_MSDU_DESC_HL;
+		else
+			ar->htt.max_num_pending_tx = TARGET_TLV_NUM_MSDU_DESC;
 		ar->wow.max_num_patterns = TARGET_TLV_NUM_WOW_PATTERNS;
 		ar->fw_stats_req_mask = WMI_STAT_PDEV | WMI_STAT_VDEV |
 			WMI_STAT_PEER;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 7c9f6f9..b870a92 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -688,6 +688,7 @@ struct ath10k_hw_ops {
 #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_WOW_PATTERNS		22
 
 /* Target specific defines for QCA9377 high latency firmware */
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 34e9770..3842caa 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1440,7 +1440,7 @@ static struct sk_buff *ath10k_wmi_tlv_op_gen_init(struct ath10k *ar)
 	cfg->rx_skip_defrag_timeout_dup_detection_check = __cpu_to_le32(0);
 	cfg->vow_config = __cpu_to_le32(0);
 	cfg->gtk_offload_max_vdev = __cpu_to_le32(2);
-	cfg->num_msdu_desc = __cpu_to_le32(TARGET_TLV_NUM_MSDU_DESC);
+	cfg->num_msdu_desc = __cpu_to_le32(ar->htt.max_num_pending_tx);
 	cfg->max_frag_entries = __cpu_to_le32(2);
 	cfg->num_tdls_vdevs = __cpu_to_le32(TARGET_TLV_NUM_TDLS_VDEVS);
 	cfg->num_tdls_conn_table_entries = __cpu_to_le32(0x20);
-- 
1.9.1

[PATCH 05/11] ath10k_sdio: use clean packet headers

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

HTC header carries junk values that may be interpreted by the firmware
differently. Enable credit update only if flow control is enabled for
the corresponding endpoint.

PLL clock setting sequence does not mask the PLL_CONTROL
register value. Side effect of not masking the values is not known as
the entire pll clock setting sequence is undocumented.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/htc.c | 4 +++-
 drivers/net/wireless/ath/ath10k/hw.c  | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 75c2a3e..23e7216 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -84,11 +84,13 @@ static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep,
 	struct ath10k_htc_hdr *hdr;
 
 	hdr = (struct ath10k_htc_hdr *)skb->data;
+	memset(hdr, 0, sizeof(struct ath10k_htc_hdr));
 
 	hdr->eid = ep->eid;
 	hdr->len = __cpu_to_le16(skb->len - sizeof(*hdr));
 	hdr->flags = 0;
-	hdr->flags |= ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE;
+	if (ep->tx_credit_flow_enabled)
+		hdr->flags |= ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE;
 
 	spin_lock_bh(&ep->htc->tx_lock);
 	hdr->seq_no = ep->seq_no++;
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index 07df7c6..2092392 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -812,6 +812,8 @@ static int ath10k_hw_qca6174_enable_pll_clock(struct ath10k *ar)
 	if (ret)
 		return -EINVAL;
 
+	reg_val &= ~(WLAN_PLL_CONTROL_REFDIV_MASK | WLAN_PLL_CONTROL_DIV_MASK |
+		     WLAN_PLL_CONTROL_NOPWD_MASK);
 	reg_val |= (SM(hw_clk->refdiv, WLAN_PLL_CONTROL_REFDIV) |
 		    SM(hw_clk->div, WLAN_PLL_CONTROL_DIV) |
 		    SM(1, WLAN_PLL_CONTROL_NOPWD));
-- 
1.9.1

[PATCH 06/11] ath10k_sdio: high latency fixes for beacon buffer

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

Beacon buffer for high latency devices does not use dma. other similar
buffer allocation methods in the driver have already been modified for
high latency path. Fix the beacon buffer allocation left out in the
earlier high latency changes.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index f374d1b..c48785b 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -944,8 +944,12 @@ static void ath10k_mac_vif_beacon_cleanup(struct ath10k_vif *arvif)
 	ath10k_mac_vif_beacon_free(arvif);
 
 	if (arvif->beacon_buf) {
-		dma_free_coherent(ar->dev, IEEE80211_MAX_FRAME_LEN,
-				  arvif->beacon_buf, arvif->beacon_paddr);
+		if (ar->is_high_latency)
+			kfree(arvif->beacon_buf);
+		else
+			dma_free_coherent(ar->dev, IEEE80211_MAX_FRAME_LEN,
+					  arvif->beacon_buf,
+					  arvif->beacon_paddr);
 		arvif->beacon_buf = NULL;
 	}
 }
@@ -5052,10 +5056,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	if (vif->type == NL80211_IFTYPE_ADHOC ||
 	    vif->type == NL80211_IFTYPE_MESH_POINT ||
 	    vif->type == NL80211_IFTYPE_AP) {
-		arvif->beacon_buf = dma_zalloc_coherent(ar->dev,
-							IEEE80211_MAX_FRAME_LEN,
-							&arvif->beacon_paddr,
-							GFP_ATOMIC);
+		if (ar->is_high_latency) {
+			arvif->beacon_buf = kmalloc(IEEE80211_MAX_FRAME_LEN,
+						    GFP_KERNEL);
+			arvif->beacon_paddr = (dma_addr_t)arvif->beacon_buf;
+		} else {
+			arvif->beacon_buf =
+				dma_zalloc_coherent(ar->dev,
+						    IEEE80211_MAX_FRAME_LEN,
+						    &arvif->beacon_paddr,
+						    GFP_ATOMIC);
+		}
 		if (!arvif->beacon_buf) {
 			ret = -ENOMEM;
 			ath10k_warn(ar, "failed to allocate beacon buffer: %d\n",
@@ -5244,8 +5255,12 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 
 err:
 	if (arvif->beacon_buf) {
-		dma_free_coherent(ar->dev, IEEE80211_MAX_FRAME_LEN,
-				  arvif->beacon_buf, arvif->beacon_paddr);
+		if (ar->is_high_latency)
+			kfree(arvif->beacon_buf);
+		else
+			dma_free_coherent(ar->dev, IEEE80211_MAX_FRAME_LEN,
+					  arvif->beacon_buf,
+					  arvif->beacon_paddr);
 		arvif->beacon_buf = NULL;
 	}
 
-- 
1.9.1

[PATCH 07/11] ath10k_sdio: fix rssi indication

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

Receive descriptor of sdio device does not include the rssi. notify
mac80211 accordingly. Without the fix, the rssi value indicated by
iw changes between the actual value and -95.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index f025363..21adcad 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1646,9 +1646,16 @@ static bool ath10k_htt_rx_proc_rx_ind_hl(struct ath10k_htt *htt,
 	hdr = (struct ieee80211_hdr *)skb->data;
 	rx_status = IEEE80211_SKB_RXCB(skb);
 	rx_status->chains |= BIT(0);
-	rx_status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
-			    rx->ppdu.combined_rssi;
-	rx_status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
+
+	if (ar->hif.bus == ATH10K_BUS_SDIO) {
+		/* SDIO firmware does not provide signal */
+		rx_status->signal = 0;
+		rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
+	} else {
+		rx_status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
+			rx->ppdu.combined_rssi;
+		rx_status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
+	}
 
 	spin_lock_bh(&ar->data_lock);
 	ch = ar->scan_channel;
-- 
1.9.1

[PATCH 08/11] ath10k_sdio: common read write

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

convert different read write functions in sdio hif to bring it under a
single read-write path. This helps in having a common dma bounce buffer
implementation. Also helps in address modification that is required
specific to change in certain mbox addresses of sdio_write.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 131 ++++++++++++++++-----------------
 1 file changed, 64 insertions(+), 67 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 77d4fa4..bb6fa67 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -36,6 +36,11 @@
 
 #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
 
+static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
+			    u32 len, bool incr);
+static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
+			     u32 len, bool incr);
+
 /* inlined helper functions */
 
 /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
@@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
 	struct sdio_func *func = ar_sdio->func;
 	unsigned char byte, asyncintdelay = 2;
 	int ret;
+	u32 addr;
 
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
 
@@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
 		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
 		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
 
-	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
-					      CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
-					      byte);
+	addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
+	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);
 	if (ret) {
 		ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
 		goto out;
@@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
 
 static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
 {
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct sdio_func *func = ar_sdio->func;
+	__le32 *buf;
 	int ret;
 
-	sdio_claim_host(func);
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
 
-	sdio_writel(func, val, addr, &ret);
+	*buf = cpu_to_le32(val);
+
+	ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);
 	if (ret) {
 		ath10k_warn(ar, "failed to write 0x%x to address 0x%x: %d\n",
 			    val, addr, ret);
@@ -250,15 +258,13 @@ static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
 		   addr, val);
 
 out:
-	sdio_release_host(func);
+	kfree(buf);
 
 	return ret;
 }
 
 static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 val)
 {
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct sdio_func *func = ar_sdio->func;
 	__le32 *buf;
 	int ret;
 
@@ -268,9 +274,7 @@ static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 val)
 
 	*buf = cpu_to_le32(val);
 
-	sdio_claim_host(func);
-
-	ret = sdio_writesb(func, addr, buf, sizeof(*buf));
+	ret = ath10k_sdio_write(ar, addr, buf, sizeof(*buf), false);
 	if (ret) {
 		ath10k_warn(ar, "failed to write value 0x%x to fixed sb address 0x%x: %d\n",
 			    val, addr, ret);
@@ -281,8 +285,6 @@ static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 val)
 		   addr, val);
 
 out:
-	sdio_release_host(func);
-
 	kfree(buf);
 
 	return ret;
@@ -290,28 +292,33 @@ static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 val)
 
 static int ath10k_sdio_read32(struct ath10k *ar, u32 addr, u32 *val)
 {
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct sdio_func *func = ar_sdio->func;
+	__le32 *buf;
 	int ret;
 
-	sdio_claim_host(func);
-	*val = sdio_readl(func, addr, &ret);
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = ath10k_sdio_read(ar, addr, buf, sizeof(*val), true);
 	if (ret) {
 		ath10k_warn(ar, "failed to read from address 0x%x: %d\n",
 			    addr, ret);
 		goto out;
 	}
 
+	*val = le32_to_cpu(*buf);
+
 	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio read32 addr 0x%x val 0x%x\n",
 		   addr, *val);
 
 out:
-	sdio_release_host(func);
+	kfree(buf);
 
 	return ret;
 }
 
-static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
+static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
+			    size_t len, bool incr)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct sdio_func *func = ar_sdio->func;
@@ -330,8 +337,12 @@ static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
 	}
 
 	sdio_claim_host(func);
+	if (incr)
+		ret = sdio_memcpy_fromio(func, tbuf, addr, len);
+	else
+		ret = sdio_readsb(func, tbuf, addr, len);
+	sdio_release_host(func);
 
-	ret = sdio_memcpy_fromio(func, tbuf, addr, len);
 	if (ret) {
 		ath10k_warn(ar, "failed to read from address 0x%x: %d\n",
 			    addr, ret);
@@ -345,14 +356,14 @@ static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
 	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL, "sdio read ", buf, len);
 
 out:
-	sdio_release_host(func);
 	if (bounced)
 		mutex_unlock(&ar_sdio->dma_buffer_mutex);
 
 	return ret;
 }
 
-static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_t len)
+static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
+			     size_t len, bool incr)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct sdio_func *func = ar_sdio->func;
@@ -371,12 +382,18 @@ static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_
 		tbuf = (u8 *)buf;
 	}
 
+	if (addr == ar_sdio->mbox_info.htc_addr)
+		addr += (ATH10K_HIF_MBOX_WIDTH - len);
+
 	sdio_claim_host(func);
 
 	/* For some reason toio() doesn't have const for the buffer, need
 	 * an ugly hack to workaround that.
 	 */
-	ret = sdio_memcpy_toio(func, addr, (void *)tbuf, len);
+	if (incr)
+		ret = sdio_memcpy_toio(func, addr, (void *)tbuf, len);
+	else
+		ret = sdio_writesb(func, addr, tbuf, len);
 	if (ret) {
 		ath10k_warn(ar, "failed to write to address 0x%x: %d\n",
 			    addr, ret);
@@ -389,39 +406,13 @@ static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_
 
 out:
 	sdio_release_host(func);
+
 	if (bounced)
 		mutex_unlock(&ar_sdio->dma_buffer_mutex);
 
 	return ret;
 }
 
-static int ath10k_sdio_readsb(struct ath10k *ar, u32 addr, void *buf, size_t len)
-{
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct sdio_func *func = ar_sdio->func;
-	int ret;
-
-	sdio_claim_host(func);
-
-	len = round_down(len, ar_sdio->mbox_info.block_size);
-
-	ret = sdio_readsb(func, buf, addr, len);
-	if (ret) {
-		ath10k_warn(ar, "failed to read from fixed (sb) address 0x%x: %d\n",
-			    addr, ret);
-		goto out;
-	}
-
-	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio readsb addr 0x%x buf 0x%p len %zu\n",
-		   addr, buf, len);
-	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL, "sdio readsb ", buf, len);
-
-out:
-	sdio_release_host(func);
-
-	return ret;
-}
-
 /* HIF mbox functions */
 
 static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
@@ -671,8 +662,8 @@ static int ath10k_sdio_mbox_rx_packet(struct ath10k *ar,
 	struct sk_buff *skb = pkt->skb;
 	int ret;
 
-	ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
-				 skb->data, pkt->alloc_len);
+	ret = ath10k_sdio_read(ar, ar_sdio->mbox_info.htc_addr,
+			       skb->data, pkt->alloc_len, false);
 	pkt->status = ret;
 	if (!ret)
 		skb_put(skb, pkt->act_len);
@@ -932,7 +923,7 @@ static int ath10k_sdio_mbox_read_int_status(struct ath10k *ar,
 	 * registers and the lookahead registers.
 	 */
 	ret = ath10k_sdio_read(ar, MBOX_HOST_INT_STATUS_ADDRESS,
-			       irq_proc_reg, sizeof(*irq_proc_reg));
+			       irq_proc_reg, sizeof(*irq_proc_reg), true);
 	if (ret)
 		goto out;
 
@@ -1177,7 +1168,8 @@ static int ath10k_sdio_bmi_exchange_msg(struct ath10k *ar,
 		addr = ar_sdio->mbox_info.htc_addr;
 
 		memcpy(ar_sdio->bmi_buf, req, req_len);
-		ret = ath10k_sdio_write(ar, addr, ar_sdio->bmi_buf, req_len);
+		ret = ath10k_sdio_write(ar, addr, ar_sdio->bmi_buf, req_len,
+					true);
 		if (ret) {
 			ath10k_warn(ar,
 				    "unable to send the bmi data to the device: %d\n",
@@ -1241,7 +1233,7 @@ static int ath10k_sdio_bmi_exchange_msg(struct ath10k *ar,
 
 	/* We always read from the start of the mbox address */
 	addr = ar_sdio->mbox_info.htc_addr;
-	ret = ath10k_sdio_read(ar, addr, ar_sdio->bmi_buf, *resp_len);
+	ret = ath10k_sdio_read(ar, addr, ar_sdio->bmi_buf, *resp_len, true);
 	if (ret) {
 		ath10k_warn(ar,
 			    "unable to read the bmi data from the device: %d\n",
@@ -1298,7 +1290,7 @@ static void __ath10k_sdio_write_async(struct ath10k *ar,
 	int ret;
 
 	skb = req->skb;
-	ret = ath10k_sdio_write(ar, req->address, skb->data, skb->len);
+	ret = ath10k_sdio_write(ar, req->address, skb->data, skb->len, true);
 	if (ret)
 		ath10k_warn(ar, "failed to write skb to 0x%x asynchronously: %d",
 			    req->address, ret);
@@ -1405,7 +1397,7 @@ static int ath10k_sdio_hif_disable_intrs(struct ath10k *ar)
 
 	memset(regs, 0, sizeof(*regs));
 	ret = ath10k_sdio_write(ar, MBOX_INT_STATUS_ENABLE_ADDRESS,
-				&regs->int_status_en, sizeof(*regs));
+				&regs->int_status_en, sizeof(*regs), true);
 	if (ret)
 		ath10k_warn(ar, "unable to disable sdio interrupts: %d\n", ret);
 
@@ -1540,7 +1532,7 @@ static int ath10k_sdio_hif_enable_intrs(struct ath10k *ar)
 			   ATH10K_SDIO_TARGET_DEBUG_INTR_MASK);
 
 	ret = ath10k_sdio_write(ar, MBOX_INT_STATUS_ENABLE_ADDRESS,
-				&regs->int_status_en, sizeof(*regs));
+				&regs->int_status_en, sizeof(*regs), true);
 	if (ret)
 		ath10k_warn(ar,
 			    "failed to update mbox interrupt status register : %d\n",
@@ -1555,7 +1547,8 @@ static int ath10k_sdio_hif_set_mbox_sleep(struct ath10k *ar, bool enable_sleep)
 	u32 val;
 	int ret;
 
-	ret = ath10k_sdio_read32(ar, ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL, &val);
+	ret = ath10k_sdio_read32(ar, ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL,
+				 &val);
 	if (ret) {
 		ath10k_warn(ar, "failed to read fifo/chip control register: %d\n",
 			    ret);
@@ -1567,7 +1560,8 @@ static int ath10k_sdio_hif_set_mbox_sleep(struct ath10k *ar, bool enable_sleep)
 	else
 		val |= ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_ON;
 
-	ret = ath10k_sdio_write32(ar, ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL, val);
+	ret = ath10k_sdio_write32(ar, ATH10K_FIFO_TIMEOUT_AND_CHIP_CONTROL,
+				  val);
 	if (ret) {
 		ath10k_warn(ar, "failed to write to FIFO_TIMEOUT_AND_CHIP_CONTROL: %d",
 			    ret);
@@ -1587,12 +1581,14 @@ static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
 	/* set window register to start read cycle */
 	ret = ath10k_sdio_write32(ar, MBOX_WINDOW_READ_ADDR_ADDRESS, address);
 	if (ret) {
-		ath10k_warn(ar, "failed to set mbox window read address: %d", ret);
+		ath10k_warn(ar, "failed to set mbox window read address: %d",
+			    ret);
 		return ret;
 	}
 
 	/* read the data */
-	ret = ath10k_sdio_read(ar, MBOX_WINDOW_DATA_ADDRESS, buf, buf_len);
+	ret = ath10k_sdio_read(ar, MBOX_WINDOW_DATA_ADDRESS, buf, buf_len,
+			       true);
 	if (ret) {
 		ath10k_warn(ar, "failed to read from mbox window data address: %d\n",
 			    ret);
@@ -1630,7 +1626,8 @@ static int ath10k_sdio_hif_diag_write_mem(struct ath10k *ar, u32 address,
 	int ret;
 
 	/* set write data */
-	ret = ath10k_sdio_write(ar, MBOX_WINDOW_DATA_ADDRESS, data, nbytes);
+	ret = ath10k_sdio_write(ar, MBOX_WINDOW_DATA_ADDRESS, data, nbytes,
+				true);
 	if (ret) {
 		ath10k_warn(ar,
 			    "failed to write 0x%p to mbox window data address: %d\n",
@@ -1641,7 +1638,7 @@ static int ath10k_sdio_hif_diag_write_mem(struct ath10k *ar, u32 address,
 	/* set window register, which starts the write cycle */
 	ret = ath10k_sdio_write32(ar, MBOX_WINDOW_WRITE_ADDR_ADDRESS, address);
 	if (ret) {
-		ath10k_warn(ar, "failed to set mbox window write address: %d", ret);
+		ath10k_warn(ar, "failed to set mbox window register: %d", ret);
 		return ret;
 	}
 
@@ -1691,7 +1688,7 @@ static int ath10k_sdio_hif_start(struct ath10k *ar)
 
 	ret = ath10k_sdio_hif_diag_read32(ar, addr, &val);
 	if (ret) {
-		ath10k_warn(ar, "unable to read hi_acs_flags address: %d\n", ret);
+		ath10k_warn(ar, "unable to read hi_acs_flags : %d\n", ret);
 		return ret;
 	}
 
-- 
1.9.1

[PATCH 09/11] ath10k_sdio: virtual scatter gather for receive

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

The existing implementation of initiating multiple sdio transfers for
receive bundling is slowing down the receive speed. Combining the
transfers using a scatter gather method would be ideal. This results in
significant performance improvement.

Since the sg implementation for sdio transfers are not reliable due to
buffer start and size alignment, a virtual scatter gather implementation
is used.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/htc.h  |   1 +
 drivers/net/wireless/ath/ath10k/sdio.c | 122 ++++++++++++++++++++++++---------
 drivers/net/wireless/ath/ath10k/sdio.h |   5 +-
 3 files changed, 93 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 24663b0..5d87908 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -58,6 +58,7 @@ enum ath10k_htc_tx_flags {
 };
 
 enum ath10k_htc_rx_flags {
+	ATH10K_HTC_FLAGS_RECV_1MORE_BLOCK = 0x01,
 	ATH10K_HTC_FLAG_TRAILER_PRESENT = 0x02,
 	ATH10K_HTC_FLAG_BUNDLE_MASK     = 0xF0
 };
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index bb6fa67..45df9db 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -35,6 +35,7 @@
 #include "sdio.h"
 
 #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
+#define ATH10K_SDIO_VSG_BUF_SIZE	(32 * 1024)
 
 static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
 			    u32 len, bool incr);
@@ -430,6 +431,7 @@ static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
 	int ret;
 
 	payload_len = le16_to_cpu(htc_hdr->len);
+	skb->len = payload_len + sizeof(struct ath10k_htc_hdr);
 
 	if (trailer_present) {
 		trailer = skb->data + sizeof(*htc_hdr) +
@@ -468,12 +470,13 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
 	enum ath10k_htc_ep_id id;
 	int ret, i, *n_lookahead_local;
 	u32 *lookaheads_local;
+	int lookahd_idx = 0;
 
 	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
 		lookaheads_local = lookaheads;
 		n_lookahead_local = n_lookahead;
 
-		id = ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
+		id = ((struct ath10k_htc_hdr *)&lookaheads[lookahd_idx++])->eid;
 
 		if (id >= ATH10K_HTC_EP_COUNT) {
 			ath10k_warn(ar, "invalid endpoint in look-ahead: %d\n",
@@ -496,6 +499,7 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
 			/* Only read lookahead's from RX trailers
 			 * for the last packet in a bundle.
 			 */
+			lookahd_idx--;
 			lookaheads_local = NULL;
 			n_lookahead_local = NULL;
 		}
@@ -529,11 +533,11 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
 	return ret;
 }
 
-static int ath10k_sdio_mbox_alloc_pkt_bundle(struct ath10k *ar,
-					     struct ath10k_sdio_rx_data *rx_pkts,
-					     struct ath10k_htc_hdr *htc_hdr,
-					     size_t full_len, size_t act_len,
-					     size_t *bndl_cnt)
+static int ath10k_sdio_mbox_alloc_bundle(struct ath10k *ar,
+					 struct ath10k_sdio_rx_data *rx_pkts,
+					 struct ath10k_htc_hdr *htc_hdr,
+					 size_t full_len, size_t act_len,
+					 size_t *bndl_cnt)
 {
 	int ret, i;
 
@@ -574,6 +578,7 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
 	size_t full_len, act_len;
 	bool last_in_bundle;
 	int ret, i;
+	int pkt_cnt = 0;
 
 	if (n_lookaheads > ATH10K_SDIO_MAX_RX_MSGS) {
 		ath10k_warn(ar,
@@ -616,16 +621,22 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
 			 * optimally fetched as a full bundle.
 			 */
 			size_t bndl_cnt;
-
-			ret = ath10k_sdio_mbox_alloc_pkt_bundle(ar,
-								&ar_sdio->rx_pkts[i],
-								htc_hdr,
-								full_len,
-								act_len,
-								&bndl_cnt);
-
-			n_lookaheads += bndl_cnt;
-			i += bndl_cnt;
+			struct ath10k_sdio_rx_data *rx_pkts =
+				&ar_sdio->rx_pkts[pkt_cnt];
+
+			ret = ath10k_sdio_mbox_alloc_bundle(ar,
+							    rx_pkts,
+							    htc_hdr,
+							    full_len,
+							    act_len,
+							    &bndl_cnt);
+
+			if (ret) {
+				ath10k_warn(ar, "alloc_bundle error %d\n", ret);
+				goto err;
+			}
+
+			pkt_cnt += bndl_cnt;
 			/*Next buffer will be the last in the bundle */
 			last_in_bundle = true;
 		}
@@ -634,14 +645,18 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
 		 * ATH10K_HTC_FLAG_BUNDLE_MASK flag set, all bundled
 		 * packet skb's have been allocated in the previous step.
 		 */
-		ret = ath10k_sdio_mbox_alloc_rx_pkt(&ar_sdio->rx_pkts[i],
+		if (htc_hdr->flags & ATH10K_HTC_FLAGS_RECV_1MORE_BLOCK)
+			full_len += ATH10K_HIF_MBOX_BLOCK_SIZE;
+
+		ret = ath10k_sdio_mbox_alloc_rx_pkt(&ar_sdio->rx_pkts[pkt_cnt],
 						    act_len,
 						    full_len,
 						    last_in_bundle,
 						    last_in_bundle);
+		pkt_cnt++;
 	}
 
-	ar_sdio->n_rx_pkts = i;
+	ar_sdio->n_rx_pkts = pkt_cnt;
 
 	return 0;
 
@@ -655,41 +670,71 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
 	return ret;
 }
 
-static int ath10k_sdio_mbox_rx_packet(struct ath10k *ar,
-				      struct ath10k_sdio_rx_data *pkt)
+static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct sk_buff *skb = pkt->skb;
+	struct ath10k_sdio_rx_data *pkt = &ar_sdio->rx_pkts[0];
+	struct sk_buff *skb;
 	int ret;
 
+	skb = pkt->skb;
 	ret = ath10k_sdio_read(ar, ar_sdio->mbox_info.htc_addr,
 			       skb->data, pkt->alloc_len, false);
-	pkt->status = ret;
-	if (!ret)
+	if (ret) {
+		ar_sdio->n_rx_pkts = 0;
+		ath10k_sdio_mbox_free_rx_pkt(pkt);
+	} else {
+		pkt->status = ret;
 		skb_put(skb, pkt->act_len);
+	}
 
 	return ret;
 }
 
-static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
+static int ath10k_sdio_mbox_rx_fetch_bundle(struct ath10k *ar)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct ath10k_sdio_rx_data *pkt;
 	int ret, i;
+	u32 pkt_offset, virt_pkt_len;
 
+	virt_pkt_len = 0;
 	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
-		ret = ath10k_sdio_mbox_rx_packet(ar,
-						 &ar_sdio->rx_pkts[i]);
-		if (ret)
+		virt_pkt_len += ar_sdio->rx_pkts[i].alloc_len;
+	}
+	if (virt_pkt_len < ATH10K_SDIO_DMA_BUF_SIZE) {
+		ret = ath10k_sdio_read(ar, ar_sdio->mbox_info.htc_addr,
+				       ar_sdio->vsg_buffer, virt_pkt_len,
+				       false);
+		if (ret) {
+			i = 0;
 			goto err;
+		}
+	} else {
+		ath10k_err(ar, "size exceeding limit %d\n", virt_pkt_len);
+	}
+
+	pkt_offset = 0;
+	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
+		struct sk_buff *skb = ar_sdio->rx_pkts[i].skb;
+
+		pkt = &ar_sdio->rx_pkts[i];
+		memcpy(skb->data, ar_sdio->vsg_buffer + pkt_offset,
+		       pkt->alloc_len);
+		pkt->status = 0;
+		skb_put(skb, pkt->act_len);
+		pkt_offset += pkt->alloc_len;
 	}
 
 	return 0;
 
 err:
 	/* Free all packets that was not successfully fetched. */
-	for (; i < ar_sdio->n_rx_pkts; i++)
+	for (i = 0; i < ar_sdio->n_rx_pkts; i++)
 		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
 
+	ar_sdio->n_rx_pkts = 0;
+
 	return ret;
 }
 
@@ -732,7 +777,10 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar,
 			 */
 			*done = false;
 
-		ret = ath10k_sdio_mbox_rx_fetch(ar);
+		if (ar_sdio->n_rx_pkts > 1)
+			ret = ath10k_sdio_mbox_rx_fetch_bundle(ar);
+		else
+			ret = ath10k_sdio_mbox_rx_fetch(ar);
 
 		/* Process fetched packets. This will potentially update
 		 * n_lookaheads depending on if the packets contain lookahead
@@ -1136,7 +1184,7 @@ static int ath10k_sdio_bmi_get_rx_lookahead(struct ath10k *ar)
 					 MBOX_HOST_INT_STATUS_ADDRESS,
 					 &rx_word);
 		if (ret) {
-			ath10k_warn(ar, "unable to read RX_LOOKAHEAD_VALID: %d\n", ret);
+			ath10k_warn(ar, "unable to read rx_lookahd: %d\n", ret);
 			return ret;
 		}
 
@@ -1480,7 +1528,7 @@ static int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
 		skb = items[i].transfer_context;
 		padded_len = ath10k_sdio_calc_txrx_padded_len(ar_sdio,
 							      skb->len);
-		skb_trim(skb, padded_len);
+		skb->len = padded_len;
 
 		/* Write TX data to the end of the mbox address space */
 		address = ar_sdio->mbox_addr[eid] + ar_sdio->mbox_size[eid] -
@@ -1508,7 +1556,8 @@ static int ath10k_sdio_hif_enable_intrs(struct ath10k *ar)
 	/* Enable all but CPU interrupts */
 	regs->int_status_en = FIELD_PREP(MBOX_INT_STATUS_ENABLE_ERROR_MASK, 1) |
 			      FIELD_PREP(MBOX_INT_STATUS_ENABLE_CPU_MASK, 1) |
-			      FIELD_PREP(MBOX_INT_STATUS_ENABLE_COUNTER_MASK, 1);
+			      FIELD_PREP(MBOX_INT_STATUS_ENABLE_COUNTER_MASK,
+					 1);
 
 	/* NOTE: There are some cases where HIF can do detection of
 	 * pending mbox messages which is disabled now.
@@ -2024,6 +2073,12 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 		goto err_free_bmi_buf;
 	}
 
+	ar_sdio->vsg_buffer = kzalloc(ATH10K_SDIO_VSG_BUF_SIZE, GFP_KERNEL);
+	if (!ar_sdio->vsg_buffer) {
+		ret = -ENOMEM;
+		goto err_free_bmi_buf;
+	}
+
 	ar_sdio->func = func;
 	sdio_set_drvdata(func, ar_sdio);
 
@@ -2081,7 +2136,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 	}
 
 	/* TODO: remove this once SDIO support is fully implemented */
-	ath10k_warn(ar, "WARNING: ath10k SDIO support is incomplete, don't expect anything to work!\n");
+	ath10k_warn(ar, "WARNING: ath10k SDIO support is experimental\n");
 
 	return 0;
 
@@ -2115,6 +2170,7 @@ static void ath10k_sdio_remove(struct sdio_func *func)
 	ath10k_core_unregister(ar);
 	ath10k_core_destroy(ar);
 	kfree(ar_sdio->dma_buffer);
+	kfree(ar_sdio->vsg_buffer);
 }
 
 static const struct sdio_device_id ath10k_sdio_devices[] = {
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index 718b8b7..8b6a86a 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -149,8 +149,8 @@ struct ath10k_sdio_irq_proc_regs {
 	u8 rx_lookahead_valid;
 	u8 host_int_status2;
 	u8 gmbox_rx_avail;
-	__le32 rx_lookahead[2];
-	__le32 rx_gmbox_lookahead_alias[2];
+	__le32 rx_lookahead[2 * ATH10K_HIF_MBOX_NUM_MAX];
+	__le32 int_status_enable;
 };
 
 struct ath10k_sdio_irq_enable_regs {
@@ -207,6 +207,7 @@ struct ath10k_sdio {
 	struct ath10k *ar;
 	struct ath10k_sdio_irq_data irq_data;
 
+	u8 *vsg_buffer;
 	u8 *dma_buffer;
 
 	/* protects access to dma_buffer */
-- 
1.9.1

[PATCH 10/11] ath10k_sdio: enable firmware crash dump

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

Handle firmware crash and gracefully restart the sdio driver on firmware
failures.  The caldata prefetch is disabled for sdio as the data read in
prefetch is resulting in errors, when enabled.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/debug.c |  3 ++
 drivers/net/wireless/ath/ath10k/sdio.c  | 86 +++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index eed4e9c..3a1982f 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1255,6 +1255,9 @@ static int ath10k_debug_cal_data_fetch(struct ath10k *ar)
 	if (WARN_ON(ar->hw_params.cal_data_len > ATH10K_DEBUG_CAL_DATA_LEN))
 		return -EINVAL;
 
+	if (ar->hif.bus == ATH10K_BUS_SDIO)
+		return -EINVAL;
+
 	hi_addr = host_interest_item_address(HI_ITEM(hi_board_data));
 
 	ret = ath10k_hif_diag_read(ar, hi_addr, &addr, sizeof(addr));
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 45df9db..11fbf6e 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -28,6 +28,7 @@
 #include "core.h"
 #include "bmi.h"
 #include "debug.h"
+#include "coredump.h"
 #include "hif.h"
 #include "htc.h"
 #include "targaddrs.h"
@@ -37,6 +38,8 @@
 #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
 #define ATH10K_SDIO_VSG_BUF_SIZE	(32 * 1024)
 
+static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
+				     size_t buf_len);
 static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
 			    u32 len, bool incr);
 static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
@@ -810,6 +813,86 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar,
 	return ret;
 }
 
+static int __ath10k_sdio_diag_read_hi(struct ath10k *ar, void *dest, u32 src,
+				      u32 len)
+{
+	u32 host_addr, addr;
+	int ret;
+
+	host_addr = host_interest_item_address(src);
+
+	ret = ath10k_sdio_hif_diag_read(ar, host_addr, &addr, sizeof(addr));
+	if (ret != 0) {
+		ath10k_warn(ar, "failed to get firmware hi address %d: %d\n",
+			    src, ret);
+		return ret;
+	}
+
+	ret = ath10k_sdio_hif_diag_read(ar, addr, dest, len);
+	if (ret != 0) {
+		ath10k_warn(ar, "failed to copy memory from %d (%d B): %d\n",
+			    addr, len, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+#define ath10k_sdio_diag_read_hi(ar, dest, src, len)		\
+	__ath10k_sdio_diag_read_hi(ar, dest, HI_ITEM(src), len)
+
+static void ath10k_sdio_dump_registers(struct ath10k *ar,
+				       struct ath10k_fw_crash_data *crash_data)
+{
+	__le32 reg_dump_values[REG_DUMP_COUNT_QCA988X] = {};
+	int i, ret;
+
+	ret = ath10k_sdio_diag_read_hi(ar, &reg_dump_values[0],
+				       hi_failure_state,
+				       sizeof(reg_dump_values));
+	if (ret) {
+		ath10k_err(ar, "failed to read firmware dump area: %d\n", ret);
+		return;
+	}
+
+	BUILD_BUG_ON(REG_DUMP_COUNT_QCA988X % 4);
+
+	ath10k_err(ar, "firmware register dump:\n");
+	for (i = 0; i < REG_DUMP_COUNT_QCA988X; i += 4)
+		ath10k_err(ar, "[%02d]: 0x%08X 0x%08X 0x%08X 0x%08X\n",
+			   i,
+			   __le32_to_cpu(reg_dump_values[i]),
+			   __le32_to_cpu(reg_dump_values[i + 1]),
+			   __le32_to_cpu(reg_dump_values[i + 2]),
+			   __le32_to_cpu(reg_dump_values[i + 3]));
+
+	if (!crash_data)
+		return;
+
+	for (i = 0; i < REG_DUMP_COUNT_QCA988X; i++)
+		crash_data->registers[i] = reg_dump_values[i];
+}
+
+static void ath10k_sdio_fw_crashed_dump(struct ath10k *ar)
+{
+	struct ath10k_fw_crash_data *crash_data;
+	char guid[UUID_STRING_LEN + 1];
+
+	ar->stats.fw_crash_counter++;
+	crash_data = ath10k_coredump_new(ar);
+
+	if (crash_data)
+		scnprintf(guid, sizeof(guid), "%pUl", &crash_data->guid);
+	else
+		scnprintf(guid, sizeof(guid), "n/a");
+
+	ath10k_err(ar, "firmware crashed! (guid %s)\n", guid);
+	ath10k_print_driver_info(ar);
+	ath10k_sdio_dump_registers(ar, crash_data);
+
+	queue_work(ar->workqueue, &ar->restart_work);
+}
+
 static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k *ar)
 {
 	u32 val;
@@ -933,6 +1016,9 @@ static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k *ar)
 		goto out;
 	}
 
+	if (cpu_int_status & 0x1)
+		ath10k_sdio_fw_crashed_dump(ar);
+
 out:
 	mutex_unlock(&irq_data->mtx);
 	return ret;
-- 
1.9.1

[PATCH 11/11] ath10k_sdio: hif start once addition

[silexcommon]

From: Alagu Sankar <alagusankar@silex-india.com>

sdio hif interface uses the start_once optimization to avoid unnecessary
firmware downloading. simulate hif_stop in sdio as core_stop does not
handle this due to start_once optimization.

Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 11fbf6e..c6f23a9 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -2255,6 +2255,12 @@ static void ath10k_sdio_remove(struct sdio_func *func)
 	cancel_work_sync(&ar_sdio->wr_async_work);
 	ath10k_core_unregister(ar);
 	ath10k_core_destroy(ar);
+
+	if (ar->is_started && ar->hw_params.start_once) {
+		ath10k_hif_stop(ar);
+		ath10k_hif_power_down(ar);
+	}
+
 	kfree(ar_sdio->dma_buffer);
 	kfree(ar_sdio->vsg_buffer);
 }
-- 
1.9.1

Re: [PATCH 02/11] ath10k_sdio: wb396 reference card fix

[Steve deRosier]

Hi Alagu,


On Sat, Sep 30, 2017 at 10:37 AM, <silexcommon@gmail.com> wrote:
>
> From: Alagu Sankar <alagusankar@silex-india.com>
>
> The QCA9377-3 WB396 sdio reference card does not get initialized
> due to the conflict in uart gpio pins. This fix is not required
> for other QCA9377 sdio cards.
>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index b4f66cd..86247c8 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -1708,8 +1708,15 @@ static int ath10k_init_uart(struct ath10k *ar)
>                 return ret;
>         }
>
> -       if (!uart_print)
> +       if (!uart_print) {
> +               /* Hack: override dbg TX pin to avoid side effects of default
> +                * GPIO_6 in QCA9377 WB396 reference card
> +                */
> +               if (ar->hif.bus == ATH10K_BUS_SDIO)
> +                       ath10k_bmi_write32(ar, hi_dbg_uart_txpin,
> +                                          ar->hw_params.uart_pin);

If it is indeed a "hack", then I don't think the maintainer should
accept this upstream. If you want it upstream you need a clean enough
implementation that doesn't need to be labeled a "hack".

Your commit message states that this is only needed for a very
specific card and not for other QCA9377 sdio cards. Yet, you're doing
this for all ATH10K_BUS_SDIO devices. Not good. I think that it's a
quirk and it's limited to a particular implementation of the device.
My suggestion: if it can be automatically determined, then do so
explicitly. If not, then it needs to be a DT setting or a module
parameter or something like that so the platform maker can decide to
do it. Having it affect all users of a SDIO QCA9377 when it doesn't
apply doesn't seem like a good idea to me.


- Steve

Re: [PATCH 02/11] ath10k_sdio: wb396 reference card fix

[Alagu Sankar]

Hi Steve,

On 2017-10-02 04:17, Steve deRosier wrote:
> Hi Alagu,
> 
> 
> On Sat, Sep 30, 2017 at 10:37 AM, <silexcommon@gmail.com> wrote:
>> 
>> From: Alagu Sankar <alagusankar@silex-india.com>
>> 
>> The QCA9377-3 WB396 sdio reference card does not get initialized
>> due to the conflict in uart gpio pins. This fix is not required
>> for other QCA9377 sdio cards.
>> 
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/core.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath10k/core.c 
>> b/drivers/net/wireless/ath/ath10k/core.c
>> index b4f66cd..86247c8 100644
>> --- a/drivers/net/wireless/ath/ath10k/core.c
>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>> @@ -1708,8 +1708,15 @@ static int ath10k_init_uart(struct ath10k *ar)
>>                 return ret;
>>         }
>> 
>> -       if (!uart_print)
>> +       if (!uart_print) {
>> +               /* Hack: override dbg TX pin to avoid side effects of 
>> default
>> +                * GPIO_6 in QCA9377 WB396 reference card
>> +                */
>> +               if (ar->hif.bus == ATH10K_BUS_SDIO)
>> +                       ath10k_bmi_write32(ar, hi_dbg_uart_txpin,
>> +                                          ar->hw_params.uart_pin);
> 
> If it is indeed a "hack", then I don't think the maintainer should
> accept this upstream. If you want it upstream you need a clean enough
> implementation that doesn't need to be labeled a "hack".

It is a hack as per the qcacld reference driver.

> Your commit message states that this is only needed for a very
> specific card and not for other QCA9377 sdio cards. Yet, you're doing
> this for all ATH10K_BUS_SDIO devices. Not good. I think that it's a
> quirk and it's limited to a particular implementation of the device.
> My suggestion: if it can be automatically determined, then do so
> explicitly. If not, then it needs to be a DT setting or a module
> parameter or something like that so the platform maker can decide to
> do it. Having it affect all users of a SDIO QCA9377 when it doesn't
> apply doesn't seem like a good idea to me.
> 
> 
> - Steve

Got it. The qcacld reference driver had it for all the QCA9377 sdio 
cards.
But we found it to be a problem only for the WB396 reference card. Will
have this checked again and release a v2 patch accordingly.

Best Regards,
Alagu Sankar

Re: [PATCH 01/11] ath10k_sdio: sdio htt data transfer fixes

[Arend van Spriel]

On 9/30/2017 7:37 PM, silexcommon@gmail.com wrote:
> From: Alagu Sankar <alagusankar@silex-india.com>
>

[...]

>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>

Not really have a specific remark for this patch, but for the entire 
series. These patches are sent using an anonymous email address, apart 
from 'silex' being in there, which does not show up in the certificate 
of origin. Just wondering if this is acceptable?

Regards,
Arend

> ---
>   drivers/net/wireless/ath/ath10k/core.c   | 20 +++++++++++++++++---
>   drivers/net/wireless/ath/ath10k/htt_rx.c |  6 ++++--
>   drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +++++++++++++++++++-----
>   3 files changed, 40 insertions(+), 10 deletions(-)

Re: [PATCH 01/11] ath10k_sdio: sdio htt data transfer fixes

[Alagu Sankar]

Hi Arend,

On 2017-10-02 13:06, Arend van Spriel wrote:
> On 9/30/2017 7:37 PM, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>> 
> 
> [...]
> 
>> 
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> 
> Not really have a specific remark for this patch, but for the entire
> series. These patches are sent using an anonymous email address, apart
> from 'silex' being in there, which does not show up in the certificate
> of origin. Just wondering if this is acceptable?
> 
> Regards,
> Arend
> 
>> ---
>>   drivers/net/wireless/ath/ath10k/core.c   | 20 +++++++++++++++++---
>>   drivers/net/wireless/ath/ath10k/htt_rx.c |  6 ++++--
>>   drivers/net/wireless/ath/ath10k/htt_tx.c | 24 
>> +++++++++++++++++++-----
>>   3 files changed, 40 insertions(+), 10 deletions(-)
> 
Could not use git send-email from the official ID due to mail server 
restrictions. If this is not acceptable, I will figure out a way to 
overcome this.

Regards,
Alagu Sankar

Re: [PATCH 00/11] SDIO support for ath10k

[Erik Stromdahl]

Hi Alagu,

It is great to see that we are finally about have fully working
mainline support for QCA9377 SDIO chipsets!

Great job!

On 2017-09-30 19:37, silexcommon@gmail.com wrote:
> From: Alagu Sankar <alagusankar@silex-india.com>
> 
> This patchset, generated against master-pending branch, enables a fully
> functional SDIO interface driver for ath10k.  Patches have been verified on
> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
> and P2P modes.
> 
> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
> with the board data from respective SDIO card vendors. Receive performance
> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
> 
Can you share any scripts etc. (wrapping hostapd and wpa_supplicant stuff)
or provide some more info about you test setup?

I made a quick socat based test on an old laptop (I don't think it has SDIO
3.0 support) and I did unfortunately not get the same figures as you did :(

> This patchset differs from the previous high latency patches, specific to SDIO.
> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
> this flag, the management frames are not sent out by the firmware. Possibility
> of management frames being sent via WMI and data frames through the reduced Tx
> completion needs to be probed further.
> 
Ah, so that explains why I couldn't see any messages in the air.

> Further improvements can be done on the transmit path by implementing packet
> bundle. Scatter Gather is another area of improvement for both Transmit and
> Receive, but may not work on all platforms
> 
> Known issues: Surprise removal of the card, when the device is in connected
> state, delays sdio function remove due to delayed WMI command failures.
> Existing ath10k framework can not differentiate between a kernel module
> removal and the surprise removal of teh card.
> 
> Alagu Sankar (11):
>    ath10k_sdio: sdio htt data transfer fixes
>    ath10k_sdio: wb396 reference card fix
>    ath10k_sdio: DMA bounce buffers for read write
>    ath10k_sdio: reduce transmit msdu count
>    ath10k_sdio: use clean packet headers
>    ath10k_sdio: high latency fixes for beacon buffer
>    ath10k_sdio: fix rssi indication
>    ath10k_sdio: common read write
>    ath10k_sdio: virtual scatter gather for receive
>    ath10k_sdio: enable firmware crash dump
>    ath10k_sdio: hif start once addition
> 
>   drivers/net/wireless/ath/ath10k/core.c    |  35 ++-
>   drivers/net/wireless/ath/ath10k/debug.c   |   3 +
>   drivers/net/wireless/ath/ath10k/htc.c     |   4 +-
>   drivers/net/wireless/ath/ath10k/htc.h     |   1 +
>   drivers/net/wireless/ath/ath10k/htt_rx.c  |  19 +-
>   drivers/net/wireless/ath/ath10k/htt_tx.c  |  24 +-
>   drivers/net/wireless/ath/ath10k/hw.c      |   2 +
>   drivers/net/wireless/ath/ath10k/hw.h      |   1 +
>   drivers/net/wireless/ath/ath10k/mac.c     |  31 ++-
>   drivers/net/wireless/ath/ath10k/sdio.c    | 398 ++++++++++++++++++++++--------
>   drivers/net/wireless/ath/ath10k/sdio.h    |  10 +-
>   drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +-
>   12 files changed, 403 insertions(+), 127 deletions(-)
> 

Re: [PATCH 02/11] ath10k_sdio: wb396 reference card fix

[Erik Stromdahl]

Hi Alagu,

On 2017-10-02 09:02, Alagu Sankar wrote:
> Hi Steve,
> 
> On 2017-10-02 04:17, Steve deRosier wrote:
>> Hi Alagu,
>>
>>
>> On Sat, Sep 30, 2017 at 10:37 AM, <silexcommon@gmail.com> wrote:
>>>
>>> From: Alagu Sankar <alagusankar@silex-india.com>
>>>
>>> The QCA9377-3 WB396 sdio reference card does not get initialized
>>> due to the conflict in uart gpio pins. This fix is not required
>>> for other QCA9377 sdio cards.
>>>
>>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>>> ---
>>>  drivers/net/wireless/ath/ath10k/core.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
>>> index b4f66cd..86247c8 100644
>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>> @@ -1708,8 +1708,15 @@ static int ath10k_init_uart(struct ath10k *ar)
>>>                 return ret;
>>>         }
>>>
>>> -       if (!uart_print)
>>> +       if (!uart_print) {
>>> +               /* Hack: override dbg TX pin to avoid side effects of default
>>> +                * GPIO_6 in QCA9377 WB396 reference card
>>> +                */
>>> +               if (ar->hif.bus == ATH10K_BUS_SDIO)
>>> +                       ath10k_bmi_write32(ar, hi_dbg_uart_txpin,
>>> +                                          ar->hw_params.uart_pin);
>>
>> If it is indeed a "hack", then I don't think the maintainer should
>> accept this upstream. If you want it upstream you need a clean enough
>> implementation that doesn't need to be labeled a "hack".
> 
> It is a hack as per the qcacld reference driver.
> 
>> Your commit message states that this is only needed for a very
>> specific card and not for other QCA9377 sdio cards. Yet, you're doing
>> this for all ATH10K_BUS_SDIO devices. Not good. I think that it's a
>> quirk and it's limited to a particular implementation of the device.
>> My suggestion: if it can be automatically determined, then do so
>> explicitly. If not, then it needs to be a DT setting or a module
>> parameter or something like that so the platform maker can decide to
>> do it. Having it affect all users of a SDIO QCA9377 when it doesn't
>> apply doesn't seem like a good idea to me.
>>
>>
>> - Steve
> 
> Got it. The qcacld reference driver had it for all the QCA9377 sdio cards.
> But we found it to be a problem only for the WB396 reference card. Will
> have this checked again and release a v2 patch accordingly.
> 
While you are at it, you might as well change the commit comments to:

"ath10k: sdio: <description>"

or perhaps just:

"ath10k: <description>"

> Best Regards,
> Alagu Sankar
> 
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 00/11] SDIO support for ath10k

[Alagu Sankar]

Hi Erik,

We will work to have this support mainlined as soon as possible. Would 
appreciate your support
in making sure that the patches do not affect the USB high latency path.

On 02-10-2017 14:32, Erik Stromdahl wrote:
> Hi Alagu,
>
> It is great to see that we are finally about have fully working
> mainline support for QCA9377 SDIO chipsets!
>
> Great job!
>
> On 2017-09-30 19:37, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>>
>> This patchset, generated against master-pending branch, enables a fully
>> functional SDIO interface driver for ath10k.  Patches have been 
>> verified on
>> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, 
>> Access Point
>> and P2P modes.
>>
>> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
>> with the board data from respective SDIO card vendors. Receive 
>> performance
>> matches the QCA reference driver when used with SDIO3.0 enabled 
>> platforms.
>> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
>>
> Can you share any scripts etc. (wrapping hostapd and wpa_supplicant 
> stuff)
> or provide some more info about you test setup?
>
I am not using any specific scripts for this. The standard ones from the 
ath10k configuration page
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/configuration 
works just fine with the
NL80211 path.
> I made a quick socat based test on an old laptop (I don't think it has 
> SDIO
> 3.0 support) and I did unfortunately not get the same figures as you 
> did :(
>
If it is SDIO v1.x, then the max bus speed is only 100Mbit/s.  With v2.x 
it is 200Mbit/s and 3.x it is
832 Mbit/s.  Throughput primarily depends on it. We used i.MX6 SoloX 
Sabre SDB platform
which supports SDIO3.x and could see the maximum performance.
>> This patchset differs from the previous high latency patches, 
>> specific to SDIO.
>> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This 
>> instructs the
>> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. 
>> Without
>> this flag, the management frames are not sent out by the firmware. 
>> Possibility
>> of management frames being sent via WMI and data frames through the 
>> reduced Tx
>> completion needs to be probed further.
>>
> Ah, so that explains why I couldn't see any messages in the air.
>
>> Further improvements can be done on the transmit path by implementing 
>> packet
>> bundle. Scatter Gather is another area of improvement for both 
>> Transmit and
>> Receive, but may not work on all platforms
>>
>> Known issues: Surprise removal of the card, when the device is in 
>> connected
>> state, delays sdio function remove due to delayed WMI command failures.
>> Existing ath10k framework can not differentiate between a kernel module
>> removal and the surprise removal of teh card.
>>
>> Alagu Sankar (11):
>>    ath10k_sdio: sdio htt data transfer fixes
>>    ath10k_sdio: wb396 reference card fix
>>    ath10k_sdio: DMA bounce buffers for read write
>>    ath10k_sdio: reduce transmit msdu count
>>    ath10k_sdio: use clean packet headers
>>    ath10k_sdio: high latency fixes for beacon buffer
>>    ath10k_sdio: fix rssi indication
>>    ath10k_sdio: common read write
>>    ath10k_sdio: virtual scatter gather for receive
>>    ath10k_sdio: enable firmware crash dump
>>    ath10k_sdio: hif start once addition
>>
>>   drivers/net/wireless/ath/ath10k/core.c    |  35 ++-
>>   drivers/net/wireless/ath/ath10k/debug.c   |   3 +
>>   drivers/net/wireless/ath/ath10k/htc.c     |   4 +-
>>   drivers/net/wireless/ath/ath10k/htc.h     |   1 +
>>   drivers/net/wireless/ath/ath10k/htt_rx.c  |  19 +-
>>   drivers/net/wireless/ath/ath10k/htt_tx.c  |  24 +-
>>   drivers/net/wireless/ath/ath10k/hw.c      |   2 +
>>   drivers/net/wireless/ath/ath10k/hw.h      |   1 +
>>   drivers/net/wireless/ath/ath10k/mac.c     |  31 ++-
>>   drivers/net/wireless/ath/ath10k/sdio.c    | 398 
>> ++++++++++++++++++++++--------
>>   drivers/net/wireless/ath/ath10k/sdio.h    |  10 +-
>>   drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +-
>>   12 files changed, 403 insertions(+), 127 deletions(-)
>>
>

Re: [PATCH 01/11] ath10k_sdio: sdio htt data transfer fixes

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 9/30/2017 7:37 PM, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>>
>
> [...]
>
>>
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>
> Not really have a specific remark for this patch, but for the entire
> series. These patches are sent using an anonymous email address, apart
> from 'silex' being in there, which does not show up in the certificate
> of origin. Just wondering if this is acceptable?

As long as there's the "correct" From header as the first line in the
commit log, which git then uses as the author instead of the From line
from email header. And I see Alagu doing that here so there shouldn't be
any problems.

--=20
Kalle Valo=

Re: [PATCH 08/11] ath10k_sdio: common read write

[Kalle Valo]

silexcommon@gmail.com writes:

> From: Alagu Sankar <alagusankar@silex-india.com>
>
> convert different read write functions in sdio hif to bring it under a
> single read-write path. This helps in having a common dma bounce buffer
> implementation. Also helps in address modification that is required
> specific to change in certain mbox addresses of sdio_write.
>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>

This didn't compile for me:

drivers/net/wireless/ath/ath10k/sdio.c:320:12: error: conflicting types for=
 'ath10k_sdio_read'
drivers/net/wireless/ath/ath10k/sdio.c:39:12: note: previous declaration of=
 'ath10k_sdio_read' was here
drivers/net/wireless/ath/ath10k/sdio.c:365:12: error: conflicting types for=
 'ath10k_sdio_write'
drivers/net/wireless/ath/ath10k/sdio.c:41:12: note: previous declaration of=
 'ath10k_sdio_write' was here
drivers/net/wireless/ath/ath10k/sdio.c:39:12: warning: 'ath10k_sdio_read' u=
sed but never defined
drivers/net/wireless/ath/ath10k/sdio.c:41:12: warning: 'ath10k_sdio_write' =
used but never defined

gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609

I fixed it like below in the pending branch. But I'll review more
carefully later, I have quite a lot of patches pending right now.

--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -37,9 +37,9 @@
 #define ATH10K_SDIO_DMA_BUF_SIZE       (32 * 1024)
=20
 static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
-                           u32 len, bool incr);
+                           size_t len, bool incr);
 static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
-                            u32 len, bool incr);
+                            size_t len, bool incr);
=20
 /* inlined helper functions */

--=20
Kalle Valo=

Re: [PATCH 00/11] SDIO support for ath10k

[Erik Stromdahl]

On 2017-10-04 08:22, Alagu Sankar wrote:
> Hi Erik,
> 
> We will work to have this support mainlined as soon as possible. Would appreciate your support
> in making sure that the patches do not affect the USB high latency path.
> 
I have added the patches in my own ath-repo and I have tested with the
WUSB6100M without any problems.

> On 02-10-2017 14:32, Erik Stromdahl wrote:
>> Hi Alagu,
>>
>> It is great to see that we are finally about have fully working
>> mainline support for QCA9377 SDIO chipsets!
>>
>> Great job!
>>
>> On 2017-09-30 19:37, silexcommon@gmail.com wrote:
>>> From: Alagu Sankar <alagusankar@silex-india.com>
>>>
>>> This patchset, generated against master-pending branch, enables a fully
>>> functional SDIO interface driver for ath10k.  Patches have been verified on
>>> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
>>> and P2P modes.
>>>
>>> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
>>> with the board data from respective SDIO card vendors. Receive performance
>>> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
>>> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
>>>
>> Can you share any scripts etc. (wrapping hostapd and wpa_supplicant stuff)
>> or provide some more info about you test setup?
>>
> I am not using any specific scripts for this. The standard ones from the ath10k configuration page
> https://wireless.wiki.kernel.org/en/users/drivers/ath10k/configuration works just fine with the
> NL80211 path.
>> I made a quick socat based test on an old laptop (I don't think it has SDIO
>> 3.0 support) and I did unfortunately not get the same figures as you did :(
>>
> If it is SDIO v1.x, then the max bus speed is only 100Mbit/s.  With v2.x it is 200Mbit/s and 3.x it is
> 832 Mbit/s.  Throughput primarily depends on it. We used i.MX6 SoloX Sabre SDB platform
> which supports SDIO3.x and could see the maximum performance.
>>> This patchset differs from the previous high latency patches, specific to SDIO.
>>> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
>>> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
>>> this flag, the management frames are not sent out by the firmware. Possibility
>>> of management frames being sent via WMI and data frames through the reduced Tx
>>> completion needs to be probed further.
>>>
>> Ah, so that explains why I couldn't see any messages in the air.
>>
>>> Further improvements can be done on the transmit path by implementing packet
>>> bundle. Scatter Gather is another area of improvement for both Transmit and
>>> Receive, but may not work on all platforms
>>>
>>> Known issues: Surprise removal of the card, when the device is in connected
>>> state, delays sdio function remove due to delayed WMI command failures.
>>> Existing ath10k framework can not differentiate between a kernel module
>>> removal and the surprise removal of teh card.
>>>
>>> Alagu Sankar (11):
>>>    ath10k_sdio: sdio htt data transfer fixes
>>>    ath10k_sdio: wb396 reference card fix
>>>    ath10k_sdio: DMA bounce buffers for read write
>>>    ath10k_sdio: reduce transmit msdu count
>>>    ath10k_sdio: use clean packet headers
>>>    ath10k_sdio: high latency fixes for beacon buffer
>>>    ath10k_sdio: fix rssi indication
>>>    ath10k_sdio: common read write
>>>    ath10k_sdio: virtual scatter gather for receive
>>>    ath10k_sdio: enable firmware crash dump
>>>    ath10k_sdio: hif start once addition
>>>
>>>   drivers/net/wireless/ath/ath10k/core.c    |  35 ++-
>>>   drivers/net/wireless/ath/ath10k/debug.c   |   3 +
>>>   drivers/net/wireless/ath/ath10k/htc.c     |   4 +-
>>>   drivers/net/wireless/ath/ath10k/htc.h     |   1 +
>>>   drivers/net/wireless/ath/ath10k/htt_rx.c  |  19 +-
>>>   drivers/net/wireless/ath/ath10k/htt_tx.c  |  24 +-
>>>   drivers/net/wireless/ath/ath10k/hw.c      |   2 +
>>>   drivers/net/wireless/ath/ath10k/hw.h      |   1 +
>>>   drivers/net/wireless/ath/ath10k/mac.c     |  31 ++-
>>>   drivers/net/wireless/ath/ath10k/sdio.c    | 398 ++++++++++++++++++++++--------
>>>   drivers/net/wireless/ath/ath10k/sdio.h    |  10 +-
>>>   drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +-
>>>   12 files changed, 403 insertions(+), 127 deletions(-)
>>>
>>
> 

Re: [PATCH 09/11] ath10k_sdio: virtual scatter gather for receive

[Erik Stromdahl]

On 2017-09-30 19:37, silexcommon@gmail.com wrote:
> From: Alagu Sankar <alagusankar@silex-india.com>
> 
> The existing implementation of initiating multiple sdio transfers for
> receive bundling is slowing down the receive speed. Combining the
> transfers using a scatter gather method would be ideal. This results in
> significant performance improvement.
> 
> Since the sg implementation for sdio transfers are not reliable due to
> buffer start and size alignment, a virtual scatter gather implementation
> is used.
> 
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> ---
>   drivers/net/wireless/ath/ath10k/htc.h  |   1 +
>   drivers/net/wireless/ath/ath10k/sdio.c | 122 ++++++++++++++++++++++++---------
>   drivers/net/wireless/ath/ath10k/sdio.h |   5 +-
>   3 files changed, 93 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
> index 24663b0..5d87908 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.h
> +++ b/drivers/net/wireless/ath/ath10k/htc.h
> @@ -58,6 +58,7 @@ enum ath10k_htc_tx_flags {
>   };
>   
>   enum ath10k_htc_rx_flags {
> +	ATH10K_HTC_FLAGS_RECV_1MORE_BLOCK = 0x01,
>   	ATH10K_HTC_FLAG_TRAILER_PRESENT = 0x02,
>   	ATH10K_HTC_FLAG_BUNDLE_MASK     = 0xF0
>   };
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
> index bb6fa67..45df9db 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -35,6 +35,7 @@
>   #include "sdio.h"
>   
>   #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
> +#define ATH10K_SDIO_VSG_BUF_SIZE	(32 * 1024)
>   
>   static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
>   			    u32 len, bool incr);
> @@ -430,6 +431,7 @@ static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
>   	int ret;
>   
>   	payload_len = le16_to_cpu(htc_hdr->len);
> +	skb->len = payload_len + sizeof(struct ath10k_htc_hdr);
>   
>   	if (trailer_present) {
>   		trailer = skb->data + sizeof(*htc_hdr) +
> @@ -468,12 +470,13 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
>   	enum ath10k_htc_ep_id id;
>   	int ret, i, *n_lookahead_local;
>   	u32 *lookaheads_local;
> +	int lookahd_idx = 0;

I think the variable should be named *lookahead_idx* instead of *lookahd_idx*,
since all other variables are using the string lookahead without abbreviations.

>   
>   	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
>   		lookaheads_local = lookaheads;
>   		n_lookahead_local = n_lookahead;
>   
> -		id = ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
> +		id = ((struct ath10k_htc_hdr *)&lookaheads[lookahd_idx++])->eid;
>   
>   		if (id >= ATH10K_HTC_EP_COUNT) {
>   			ath10k_warn(ar, "invalid endpoint in look-ahead: %d\n",
> @@ -496,6 +499,7 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
>   			/* Only read lookahead's from RX trailers
>   			 * for the last packet in a bundle.
>   			 */
> +			lookahd_idx--;
>   			lookaheads_local = NULL;
>   			n_lookahead_local = NULL;
>   		}
> @@ -529,11 +533,11 @@ static int ath10k_sdio_mbox_rx_process_packets(struct ath10k *ar,
>   	return ret;
>   }
>   
> -static int ath10k_sdio_mbox_alloc_pkt_bundle(struct ath10k *ar,
> -					     struct ath10k_sdio_rx_data *rx_pkts,
> -					     struct ath10k_htc_hdr *htc_hdr,
> -					     size_t full_len, size_t act_len,
> -					     size_t *bndl_cnt)
> +static int ath10k_sdio_mbox_alloc_bundle(struct ath10k *ar,
> +					 struct ath10k_sdio_rx_data *rx_pkts,
> +					 struct ath10k_htc_hdr *htc_hdr,
> +					 size_t full_len, size_t act_len,
> +					 size_t *bndl_cnt)
>   {
>   	int ret, i;
>   
> @@ -574,6 +578,7 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
>   	size_t full_len, act_len;
>   	bool last_in_bundle;
>   	int ret, i;
> +	int pkt_cnt = 0;
>   
>   	if (n_lookaheads > ATH10K_SDIO_MAX_RX_MSGS) {
>   		ath10k_warn(ar,
> @@ -616,16 +621,22 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
>   			 * optimally fetched as a full bundle.
>   			 */
>   			size_t bndl_cnt;
> -
> -			ret = ath10k_sdio_mbox_alloc_pkt_bundle(ar,
> -								&ar_sdio->rx_pkts[i],
> -								htc_hdr,
> -								full_len,
> -								act_len,
> -								&bndl_cnt);
> -
> -			n_lookaheads += bndl_cnt;
> -			i += bndl_cnt;
> +			struct ath10k_sdio_rx_data *rx_pkts =
> +				&ar_sdio->rx_pkts[pkt_cnt];
> +
> +			ret = ath10k_sdio_mbox_alloc_bundle(ar,
> +							    rx_pkts,
> +							    htc_hdr,
> +							    full_len,
> +							    act_len,
> +							    &bndl_cnt);
> +
> +			if (ret) {
> +				ath10k_warn(ar, "alloc_bundle error %d\n", ret);
> +				goto err;
> +			}
> +
> +			pkt_cnt += bndl_cnt;
>   			/*Next buffer will be the last in the bundle */
>   			last_in_bundle = true;
>   		}
> @@ -634,14 +645,18 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
>   		 * ATH10K_HTC_FLAG_BUNDLE_MASK flag set, all bundled
>   		 * packet skb's have been allocated in the previous step.
>   		 */
> -		ret = ath10k_sdio_mbox_alloc_rx_pkt(&ar_sdio->rx_pkts[i],
> +		if (htc_hdr->flags & ATH10K_HTC_FLAGS_RECV_1MORE_BLOCK)
> +			full_len += ATH10K_HIF_MBOX_BLOCK_SIZE;
> +
> +		ret = ath10k_sdio_mbox_alloc_rx_pkt(&ar_sdio->rx_pkts[pkt_cnt],
>   						    act_len,
>   						    full_len,
>   						    last_in_bundle,
>   						    last_in_bundle);
> +		pkt_cnt++;
>   	}
>   
> -	ar_sdio->n_rx_pkts = i;
> +	ar_sdio->n_rx_pkts = pkt_cnt;
>   
>   	return 0;
>   
> @@ -655,41 +670,71 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
>   	return ret;
>   }
>   
> -static int ath10k_sdio_mbox_rx_packet(struct ath10k *ar,
> -				      struct ath10k_sdio_rx_data *pkt)
> +static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
>   {
>   	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> -	struct sk_buff *skb = pkt->skb;
> +	struct ath10k_sdio_rx_data *pkt = &ar_sdio->rx_pkts[0];
> +	struct sk_buff *skb;
>   	int ret;
>   
> +	skb = pkt->skb;
>   	ret = ath10k_sdio_read(ar, ar_sdio->mbox_info.htc_addr,
>   			       skb->data, pkt->alloc_len, false);
> -	pkt->status = ret;
> -	if (!ret)
> +	if (ret) {
> +		ar_sdio->n_rx_pkts = 0;
> +		ath10k_sdio_mbox_free_rx_pkt(pkt);
> +	} else {
> +		pkt->status = ret;
>   		skb_put(skb, pkt->act_len);
> +	}
>   
>   	return ret;
>   }
>   
> -static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
> +static int ath10k_sdio_mbox_rx_fetch_bundle(struct ath10k *ar)
>   {
>   	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> +	struct ath10k_sdio_rx_data *pkt;
>   	int ret, i;
> +	u32 pkt_offset, virt_pkt_len;
>   
> +	virt_pkt_len = 0;
>   	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
> -		ret = ath10k_sdio_mbox_rx_packet(ar,
> -						 &ar_sdio->rx_pkts[i]);
> -		if (ret)
> +		virt_pkt_len += ar_sdio->rx_pkts[i].alloc_len;
> +	}
> +	if (virt_pkt_len < ATH10K_SDIO_DMA_BUF_SIZE) {
> +		ret = ath10k_sdio_read(ar, ar_sdio->mbox_info.htc_addr,
> +				       ar_sdio->vsg_buffer, virt_pkt_len,
> +				       false);
> +		if (ret) {
> +			i = 0;
>   			goto err;
> +		}
> +	} else {
> +		ath10k_err(ar, "size exceeding limit %d\n", virt_pkt_len);
> +	}
> +
> +	pkt_offset = 0;
> +	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
> +		struct sk_buff *skb = ar_sdio->rx_pkts[i].skb;
> +
> +		pkt = &ar_sdio->rx_pkts[i];
> +		memcpy(skb->data, ar_sdio->vsg_buffer + pkt_offset,
> +		       pkt->alloc_len);
> +		pkt->status = 0;
> +		skb_put(skb, pkt->act_len);
> +		pkt_offset += pkt->alloc_len;
>   	}
>   
>   	return 0;
>   
>   err:
>   	/* Free all packets that was not successfully fetched. */

Change comment to: /* Free all packets */
since all packets are freed and not only those that was not successfully fetched.

> -	for (; i < ar_sdio->n_rx_pkts; i++)
> +	for (i = 0; i < ar_sdio->n_rx_pkts; i++)
>   		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
>   
> +	ar_sdio->n_rx_pkts = 0;
> +
>   	return ret;
>   }
>   
> @@ -732,7 +777,10 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar,
>   			 */
>   			*done = false;
>   
> -		ret = ath10k_sdio_mbox_rx_fetch(ar);
> +		if (ar_sdio->n_rx_pkts > 1)
> +			ret = ath10k_sdio_mbox_rx_fetch_bundle(ar);
> +		else
> +			ret = ath10k_sdio_mbox_rx_fetch(ar);

ret is not checked at all (I noticed this is the case in the current code as well).
I think it would be wise to break the loop if error:

if (ret)
	break;

>   
>   		/* Process fetched packets. This will potentially update
>   		 * n_lookaheads depending on if the packets contain lookahead
> @@ -1136,7 +1184,7 @@ static int ath10k_sdio_bmi_get_rx_lookahead(struct ath10k *ar)
>   					 MBOX_HOST_INT_STATUS_ADDRESS,
>   					 &rx_word);
>   		if (ret) {
> -			ath10k_warn(ar, "unable to read RX_LOOKAHEAD_VALID: %d\n", ret);
> +			ath10k_warn(ar, "unable to read rx_lookahd: %d\n", ret);

Change print to "unable to read RX lookahead: %d\n" as it is more descriptive

>   			return ret;
>   		}
>   
> @@ -1480,7 +1528,7 @@ static int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
>   		skb = items[i].transfer_context;
>   		padded_len = ath10k_sdio_calc_txrx_padded_len(ar_sdio,
>   							      skb->len);
> -		skb_trim(skb, padded_len);
> +		skb->len = padded_len;

Why this change?
I think the skb_ family of functions is the preferred way to manipulate skb's

>   
>   		/* Write TX data to the end of the mbox address space */
>   		address = ar_sdio->mbox_addr[eid] + ar_sdio->mbox_size[eid] -
> @@ -1508,7 +1556,8 @@ static int ath10k_sdio_hif_enable_intrs(struct ath10k *ar)
>   	/* Enable all but CPU interrupts */
>   	regs->int_status_en = FIELD_PREP(MBOX_INT_STATUS_ENABLE_ERROR_MASK, 1) |
>   			      FIELD_PREP(MBOX_INT_STATUS_ENABLE_CPU_MASK, 1) |
> -			      FIELD_PREP(MBOX_INT_STATUS_ENABLE_COUNTER_MASK, 1);
> +			      FIELD_PREP(MBOX_INT_STATUS_ENABLE_COUNTER_MASK,
> +					 1);

Is this a checkpatch-fix?
I would recommend creating a separate patch for style issues.

>   
>   	/* NOTE: There are some cases where HIF can do detection of
>   	 * pending mbox messages which is disabled now.
> @@ -2024,6 +2073,12 @@ static int ath10k_sdio_probe(struct sdio_func *func,
>   		goto err_free_bmi_buf;
>   	}
>   
> +	ar_sdio->vsg_buffer = kzalloc(ATH10K_SDIO_VSG_BUF_SIZE, GFP_KERNEL);
> +	if (!ar_sdio->vsg_buffer) {
> +		ret = -ENOMEM;
> +		goto err_free_bmi_buf;
> +	}
> +
>   	ar_sdio->func = func;
>   	sdio_set_drvdata(func, ar_sdio);
>   
> @@ -2081,7 +2136,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
>   	}
>   
>   	/* TODO: remove this once SDIO support is fully implemented */
> -	ath10k_warn(ar, "WARNING: ath10k SDIO support is incomplete, don't expect anything to work!\n");
> +	ath10k_warn(ar, "WARNING: ath10k SDIO support is experimental\n");
>   
>   	return 0;
>   
> @@ -2115,6 +2170,7 @@ static void ath10k_sdio_remove(struct sdio_func *func)
>   	ath10k_core_unregister(ar);
>   	ath10k_core_destroy(ar);
>   	kfree(ar_sdio->dma_buffer);
> +	kfree(ar_sdio->vsg_buffer);
>   }
>   
>   static const struct sdio_device_id ath10k_sdio_devices[] = {
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
> index 718b8b7..8b6a86a 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.h
> +++ b/drivers/net/wireless/ath/ath10k/sdio.h
> @@ -149,8 +149,8 @@ struct ath10k_sdio_irq_proc_regs {
>   	u8 rx_lookahead_valid;
>   	u8 host_int_status2;
>   	u8 gmbox_rx_avail;
> -	__le32 rx_lookahead[2];
> -	__le32 rx_gmbox_lookahead_alias[2];
> +	__le32 rx_lookahead[2 * ATH10K_HIF_MBOX_NUM_MAX];
> +	__le32 int_status_enable;
>   };
>   
>   struct ath10k_sdio_irq_enable_regs {
> @@ -207,6 +207,7 @@ struct ath10k_sdio {
>   	struct ath10k *ar;
>   	struct ath10k_sdio_irq_data irq_data;
>   
> +	u8 *vsg_buffer;
>   	u8 *dma_buffer;
>   
>   	/* protects access to dma_buffer */
> 

Re: [08/11] ath10k_sdio: common read write

[Gary Bisson]

Hi Alagu,

On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcommon@gmail.com wrote:
> From: Alagu Sankar <alagusankar@silex-india.com>
> 
> convert different read write functions in sdio hif to bring it under a
> single read-write path. This helps in having a common dma bounce buffer
> implementation. Also helps in address modification that is required
> specific to change in certain mbox addresses of sdio_write.
> 
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> ---
>  drivers/net/wireless/ath/ath10k/sdio.c | 131 ++++++++++++++++-----------------
>  1 file changed, 64 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
> index 77d4fa4..bb6fa67 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -36,6 +36,11 @@
>  
>  #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
>  
> +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
> +			    u32 len, bool incr);
> +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf,
> +			     u32 len, bool incr);
> +

As mentioned by Kalle, u32 needs to be size_t.

>  /* inlined helper functions */
>  
>  /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
> @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
>  	struct sdio_func *func = ar_sdio->func;
>  	unsigned char byte, asyncintdelay = 2;
>  	int ret;
> +	u32 addr;
>  
>  	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
>  
> @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
>  		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
>  		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
>  
> -	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
> -					      CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> -					      byte);
> +	addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> +	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);

Not sure this part is needed.

>  	if (ret) {
>  		ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
>  		goto out;
> @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
>  
>  static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
>  {
> -	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> -	struct sdio_func *func = ar_sdio->func;
> +	__le32 *buf;
>  	int ret;
>  
> -	sdio_claim_host(func);
> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
>  
> -	sdio_writel(func, val, addr, &ret);
> +	*buf = cpu_to_le32(val);
> +
> +	ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);

Shouldn't we use buf instead of val? buf seems pretty useless otherwise.

Regards,
Gary

Re: [PATCH 00/11] SDIO support for ath10k

[Gary Bisson]

Hi Alagu,

First of all, thank you for sharing your patches, this will be a very
nice improvement to have SDIO QCA9377 working with ath10k.

I've tried your series with Nitrogen7 [1] platform which is supported in
mainline already. It uses BD-SDMAC [2] which uses the same module as the
SX-SDMAC.

Below are some questions/remarks I have after the testing.

On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote:
> From: Alagu Sankar <alagusankar at silex-india.com>
> 
> This patchset, generated against master-pending branch, enables a fully
> functional SDIO interface driver for ath10k.  Patches have been verified on
> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
> and P2P modes.
> 
> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1

Quick question on the firmware, is it the one from Kalle's repository?[3]

If so, where does this firmware comes from? Is 00061 the firmware
version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output:
Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1

> with the board data from respective SDIO card vendors.

About the board-sdio.bin, is it just a copy of your bdwlan30.bin?

> Receive performance
> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s

Nice performances. Unfortunately I don't get better than 30Mbits/s in TX
and 50Mbits/s in RX:
# iperf -c 192.168.1.1    
------------------------------------------------------------
Client connecting to 192.168.1.1, TCP port 5001
TCP window size: 43.8 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port
5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0-10.0 sec  34.9 MBytes  29.2 Mbits/sec
# iperf -s
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
[  4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port
50646
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0-10.0 sec  63.1 MBytes  52.7 Mbits/sec

Do you have any idea why? Note that qcacld-2.0 driver on that same
platform (same OS) gives the performances you advertize (150Mbits/s).

> This patchset differs from the previous high latency patches, specific to SDIO.
> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
> this flag, the management frames are not sent out by the firmware. Possibility
> of management frames being sent via WMI and data frames through the reduced Tx
> completion needs to be probed further.
> 
> Further improvements can be done on the transmit path by implementing packet
> bundle. Scatter Gather is another area of improvement for both Transmit and
> Receive, but may not work on all platforms
> 
> Known issues: Surprise removal of the card, when the device is in connected
> state, delays sdio function remove due to delayed WMI command failures.
> Existing ath10k framework can not differentiate between a kernel module
> removae and the surprise removal of teh card.

Here are some questions:
- Is it normal to see a warning about board-2.bin, shouldn't it look for
  board-sdio.bin only?
[   14.696704] ath10k_sdio mmc1:0001:1: Direct firmware load for
ath10k/QCA9377/hw1.0/board-2.bin failed with error -2

- Did you have pre-cal and/or cal binaries for your testing?
[   14.067159] ath10k_sdio mmc1:0001:1: Direct firmware load for
ath10k/pre-cal-sdio-mmc1:0001:1.bin failed with error -2
[   14.149257] ath10k_sdio mmc1:0001:1: Direct firmware load for
ath10k/cal-sdio-mmc1:0001:1.bin failed with error -2

Also noticed that the "tx bitrate" output of 'iw link' is wrong in my
case:
# iw wlan0 link
Connected to 00:00:00:00:00:b0 (on wlan0)
	SSID: TPLINK_AC_5G
	freq: 5180
	RX: 72072365 bytes (67934 packets)
	TX: 79084128 bytes (73649 packets)
	signal: -35 dBm
	tx bitrate: 6.0 MBit/s

	bss flags:	short-slot-time
	dtim period:	2
	beacon int:	100

When connecting using qcacld driver it shows 433MBit/s as expected. Is
it working properly in your case?

As a FYI, I've built Erik's tree[4] for this testing, should I use
another tree instead?

Let me know your thoughts.

Regards,
Gary

[1] https://boundarydevices.com/product/nitrogen7/
[2] https://boundarydevices.com/product/bd_sdmac_wifi/
[3] https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested
[4] https://github.com/erstrom/linux-ath

Re: [PATCH 00/11] SDIO support for ath10k

[Alagu Sankar]

Hi Gary,

On 05-10-2017 20:42, Gary Bisson wrote:
> Hi Alagu,
>
> First of all, thank you for sharing your patches, this will be a very
> nice improvement to have SDIO QCA9377 working with ath10k.
>
> I've tried your series with Nitrogen7 [1] platform which is supported in
> mainline already. It uses BD-SDMAC [2] which uses the same module as the
> SX-SDMAC.
>
> Below are some questions/remarks I have after the testing.
>
> On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote:
>> From: Alagu Sankar <alagusankar at silex-india.com>
>>
>> This patchset, generated against master-pending branch, enables a fully
>> functional SDIO interface driver for ath10k.  Patches have been verified on
>> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
>> and P2P modes.
>>
>> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
> Quick question on the firmware, is it the one from Kalle's repository?[3]
>
> If so, where does this firmware comes from? Is 00061 the firmware
> version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output:
> Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1
Yes, it is from 
https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested. 
I have also used custom firmware from QCA/Silex as used in qcacld-2.0 
driver without any issue. You need to use the firmware merger tool from 
https://github.com/erstrom/linux-ath/wiki/Firmware to combine the 
qwlan30.bin and otp30.bin to generate the firmware-sdio.bin.
>> with the board data from respective SDIO card vendors.
> About the board-sdio.bin, is it just a copy of your bdwlan30.bin?
Yes board-sdio.bin is a copy of bdwlan30.bin
>> Receive performance
>> matches the QCA reference driver when used with SDIO3.0 enabled platforms.
>> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
> Nice performances. Unfortunately I don't get better than 30Mbits/s in TX
> and 50Mbits/s in RX:
> # iperf -c 192.168.1.1
> ------------------------------------------------------------
> Client connecting to 192.168.1.1, TCP port 5001
> TCP window size: 43.8 KByte (default)
> ------------------------------------------------------------
> [  3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port
> 5001
> [ ID] Interval       Transfer     Bandwidth
> [  3]  0.0-10.0 sec  34.9 MBytes  29.2 Mbits/sec
> # iperf -s
> ------------------------------------------------------------
> Server listening on TCP port 5001
> TCP window size: 85.3 KByte (default)
> ------------------------------------------------------------
> [  4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port
> 50646
> [ ID] Interval       Transfer     Bandwidth
> [  4]  0.0-10.0 sec  63.1 MBytes  52.7 Mbits/sec
>
> Do you have any idea why? Note that qcacld-2.0 driver on that same
> platform (same OS) gives the performances you advertize (150Mbits/s).
For some reason, if I use the imx_v6_v7_defconfig as is, performance is 
very poor. In fact, the firmware download itself will take about 6 
seconds. This can also be seen when you up/down the wlan0 interface. For 
the i.MX6 SoloX board, I used the configuration of 4.1.15 as provided by 
the BSP from NXP/Freescale.  This improved the performance quite a bit. 
I haven't had a chance to spend time on this to figure out the 
difference and reason. Another difference is that the default device 
tree for SoloX did not have the correct settings to support SDIO3.x.  
Had to modify them, but did not include the device tree patches here as 
it is not meant for this group.
>> This patchset differs from the previous high latency patches, specific to SDIO.
>> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
>> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
>> this flag, the management frames are not sent out by the firmware. Possibility
>> of management frames being sent via WMI and data frames through the reduced Tx
>> completion needs to be probed further.
>>
>> Further improvements can be done on the transmit path by implementing packet
>> bundle. Scatter Gather is another area of improvement for both Transmit and
>> Receive, but may not work on all platforms
>>
>> Known issues: Surprise removal of the card, when the device is in connected
>> state, delays sdio function remove due to delayed WMI command failures.
>> Existing ath10k framework can not differentiate between a kernel module
>> removae and the surprise removal of teh card.
> Here are some questions:
> - Is it normal to see a warning about board-2.bin, shouldn't it look for
>    board-sdio.bin only?
> [   14.696704] ath10k_sdio mmc1:0001:1: Direct firmware load for
> ath10k/QCA9377/hw1.0/board-2.bin failed with error -2
This was only noticed in the latest update. Like the different firmware 
versions, the driver also looks for the board-2.bin now. I have only 
tested with the board-sdio.bin
> - Did you have pre-cal and/or cal binaries for your testing?
> [   14.067159] ath10k_sdio mmc1:0001:1: Direct firmware load for
> ath10k/pre-cal-sdio-mmc1:0001:1.bin failed with error -2
> [   14.149257] ath10k_sdio mmc1:0001:1: Direct firmware load for
> ath10k/cal-sdio-mmc1:0001:1.bin failed with error -2
No. I did not use the pre-cal and cal binaries.
> Also noticed that the "tx bitrate" output of 'iw link' is wrong in my
> case:
> # iw wlan0 link
> Connected to 00:00:00:00:00:b0 (on wlan0)
> 	SSID: TPLINK_AC_5G
> 	freq: 5180
> 	RX: 72072365 bytes (67934 packets)
> 	TX: 79084128 bytes (73649 packets)
> 	signal: -35 dBm
> 	tx bitrate: 6.0 MBit/s
>
> 	bss flags:	short-slot-time
> 	dtim period:	2
> 	beacon int:	100
>
> When connecting using qcacld driver it shows 433MBit/s as expected. Is
> it working properly in your case?
Tx rate is not updated properly.  I will include it in the list of known 
issues.
> As a FYI, I've built Erik's tree[4] for this testing, should I use
> another tree instead?
I use the Kalle's ath10k tree, but when I last looked, they were pretty 
much the same, so it should not be a problem.
> Let me know your thoughts.
>
> Regards,
> Gary
>
> [1] https://boundarydevices.com/product/nitrogen7/
> [2] https://boundarydevices.com/product/bd_sdmac_wifi/
> [3] https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested
> [4] https://github.com/erstrom/linux-ath
>
Regards,
Alagu

Re: [08/11] ath10k_sdio: common read write

[Alagu Sankar]

Hi Gary,


On 2017-10-05 15:39, Gary Bisson wrote:
> Hi Alagu,
> 
> On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>> 
>> convert different read write functions in sdio hif to bring it under a
>> single read-write path. This helps in having a common dma bounce 
>> buffer
>> implementation. Also helps in address modification that is required
>> specific to change in certain mbox addresses of sdio_write.
>> 
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/sdio.c | 131 
>> ++++++++++++++++-----------------
>>  1 file changed, 64 insertions(+), 67 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c 
>> b/drivers/net/wireless/ath/ath10k/sdio.c
>> index 77d4fa4..bb6fa67 100644
>> --- a/drivers/net/wireless/ath/ath10k/sdio.c
>> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
>> @@ -36,6 +36,11 @@
>> 
>>  #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
>> 
>> +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
>> +			    u32 len, bool incr);
>> +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void 
>> *buf,
>> +			     u32 len, bool incr);
>> +
> 
> As mentioned by Kalle, u32 needs to be size_t.
Yes, the compiler I used is probably a step older and did not catch 
this.
> 
>>  /* inlined helper functions */
>> 
>>  /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
>> @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
>>  	struct sdio_func *func = ar_sdio->func;
>>  	unsigned char byte, asyncintdelay = 2;
>>  	int ret;
>> +	u32 addr;
>> 
>>  	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
>> 
>> @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
>>  		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
>>  		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
>> 
>> -	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
>> -					      CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
>> -					      byte);
>> +	addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
>> +	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);
> 
> Not sure this part is needed.
This is to overcome checkpatch warning. Too big a names for the function 
and macro
not helping in there. Will have to move it as a separate patch.
> 
>>  	if (ret) {
>>  		ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
>>  		goto out;
>> @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
>> 
>>  static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
>>  {
>> -	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> -	struct sdio_func *func = ar_sdio->func;
>> +	__le32 *buf;
>>  	int ret;
>> 
>> -	sdio_claim_host(func);
>> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>> +	if (!buf)
>> +		return -ENOMEM;
>> 
>> -	sdio_writel(func, val, addr, &ret);
>> +	*buf = cpu_to_le32(val);
>> +
>> +	ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);
> 
> Shouldn't we use buf instead of val? buf seems pretty useless 
> otherwise.
Yes, thanks for pointing this out. will be corrected in v2.
> 
> Regards,
> Gary
> 
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 00/11] SDIO support for ath10k

[Gary Bisson]

Hi Alagu,

On Thu, Oct 05, 2017 at 10:54:26PM +0530, Alagu Sankar wrote:
> Hi Gary,
> 
> On 05-10-2017 20:42, Gary Bisson wrote:
> > Hi Alagu,
> > 
> > First of all, thank you for sharing your patches, this will be a very
> > nice improvement to have SDIO QCA9377 working with ath10k.
> > 
> > I've tried your series with Nitrogen7 [1] platform which is supported in
> > mainline already. It uses BD-SDMAC [2] which uses the same module as the
> > SX-SDMAC.
> > 
> > Below are some questions/remarks I have after the testing.
> > 
> > On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote:
> > > From: Alagu Sankar <alagusankar at silex-india.com>
> > > 
> > > This patchset, generated against master-pending branch, enables a fully
> > > functional SDIO interface driver for ath10k.  Patches have been verified on
> > > QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
> > > and P2P modes.
> > > 
> > > The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
> > Quick question on the firmware, is it the one from Kalle's repository?[3]
> > 
> > If so, where does this firmware comes from? Is 00061 the firmware
> > version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output:
> > Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1
> Yes, it is from
> https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested.
> I have also used custom firmware from QCA/Silex as used in qcacld-2.0 driver
> without any issue. You need to use the firmware merger tool from
> https://github.com/erstrom/linux-ath/wiki/Firmware to combine the
> qwlan30.bin and otp30.bin to generate the firmware-sdio.bin.

Good to know, thanks. Maybe Kalle can tell us more about the firmware
itself, what's the difference between the version 0.0.0.60 and 0.0.0.61?

> > > with the board data from respective SDIO card vendors.
> > About the board-sdio.bin, is it just a copy of your bdwlan30.bin?
> Yes board-sdio.bin is a copy of bdwlan30.bin

Thanks for confirming it.

> > > Receive performance
> > > matches the QCA reference driver when used with SDIO3.0 enabled platforms.
> > > iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
> > Nice performances. Unfortunately I don't get better than 30Mbits/s in TX
> > and 50Mbits/s in RX:
> > # iperf -c 192.168.1.1
> > ------------------------------------------------------------
> > Client connecting to 192.168.1.1, TCP port 5001
> > TCP window size: 43.8 KByte (default)
> > ------------------------------------------------------------
> > [  3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port
> > 5001
> > [ ID] Interval       Transfer     Bandwidth
> > [  3]  0.0-10.0 sec  34.9 MBytes  29.2 Mbits/sec
> > # iperf -s
> > ------------------------------------------------------------
> > Server listening on TCP port 5001
> > TCP window size: 85.3 KByte (default)
> > ------------------------------------------------------------
> > [  4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port
> > 50646
> > [ ID] Interval       Transfer     Bandwidth
> > [  4]  0.0-10.0 sec  63.1 MBytes  52.7 Mbits/sec
> > 
> > Do you have any idea why? Note that qcacld-2.0 driver on that same
> > platform (same OS) gives the performances you advertize (150Mbits/s).
> For some reason, if I use the imx_v6_v7_defconfig as is, performance is very
> poor. In fact, the firmware download itself will take about 6 seconds. This
> can also be seen when you up/down the wlan0 interface. For the i.MX6 SoloX
> board, I used the configuration of 4.1.15 as provided by the BSP from
> NXP/Freescale.

Do you mean that you've used 4.1.15 kernel or just the 4.1.15
configuration on latest 4.14?

> This improved the performance quite a bit. I haven't had a
> chance to spend time on this to figure out the difference and reason.
> Another difference is that the default device tree for SoloX did not have
> the correct settings to support SDIO3.x.  Had to modify them, but did not
> include the device tree patches here as it is not meant for this group.

Doh! That's why I don't get better performances, my platform limits the
SDIO bus to 50MHz right now.

Unfortunately SDIO3.0 doesn't seem stable on i.MX7, it is missing a few
patches, I'll start a thread with Shawn/Fabio about it.

> > > This patchset differs from the previous high latency patches, specific to SDIO.
> > > HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instructs the
> > > firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Without
> > > this flag, the management frames are not sent out by the firmware. Possibility
> > > of management frames being sent via WMI and data frames through the reduced Tx
> > > completion needs to be probed further.
> > > 
> > > Further improvements can be done on the transmit path by implementing packet
> > > bundle. Scatter Gather is another area of improvement for both Transmit and
> > > Receive, but may not work on all platforms
> > > 
> > > Known issues: Surprise removal of the card, when the device is in connected
> > > state, delays sdio function remove due to delayed WMI command failures.
> > > Existing ath10k framework can not differentiate between a kernel module
> > > removae and the surprise removal of teh card.
> > Here are some questions:
> > - Is it normal to see a warning about board-2.bin, shouldn't it look for
> >    board-sdio.bin only?
> > [   14.696704] ath10k_sdio mmc1:0001:1: Direct firmware load for
> > ath10k/QCA9377/hw1.0/board-2.bin failed with error -2
> This was only noticed in the latest update. Like the different firmware
> versions, the driver also looks for the board-2.bin now. I have only tested
> with the board-sdio.bin

Good, thanks.

> > - Did you have pre-cal and/or cal binaries for your testing?
> > [   14.067159] ath10k_sdio mmc1:0001:1: Direct firmware load for
> > ath10k/pre-cal-sdio-mmc1:0001:1.bin failed with error -2
> > [   14.149257] ath10k_sdio mmc1:0001:1: Direct firmware load for
> > ath10k/cal-sdio-mmc1:0001:1.bin failed with error -2
> No. I did not use the pre-cal and cal binaries.
> > Also noticed that the "tx bitrate" output of 'iw link' is wrong in my
> > case:
> > # iw wlan0 link
> > Connected to 00:00:00:00:00:b0 (on wlan0)
> > 	SSID: TPLINK_AC_5G
> > 	freq: 5180
> > 	RX: 72072365 bytes (67934 packets)
> > 	TX: 79084128 bytes (73649 packets)
> > 	signal: -35 dBm
> > 	tx bitrate: 6.0 MBit/s
> > 
> > 	bss flags:	short-slot-time
> > 	dtim period:	2
> > 	beacon int:	100
> > 
> > When connecting using qcacld driver it shows 433MBit/s as expected. Is
> > it working properly in your case?
> Tx rate is not updated properly.  I will include it in the list of known
> issues.

I'll try to see if it is complex to fix, I had a similar issue on qcacld
which was straightforward to fix.

Regards,
Gary

Re: [08/11] ath10k_sdio: common read write

[Gary Bisson]

Hi Alagu,

On Thu, Oct 05, 2017 at 11:03:12PM +0530, Alagu Sankar wrote:
> Hi Gary,
> 
> 
> On 2017-10-05 15:39, Gary Bisson wrote:
> > Hi Alagu,
> > 
> > On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcommon@gmail.com wrote:
> > > From: Alagu Sankar <alagusankar@silex-india.com>
> > > 
> > > convert different read write functions in sdio hif to bring it under a
> > > single read-write path. This helps in having a common dma bounce
> > > buffer
> > > implementation. Also helps in address modification that is required
> > > specific to change in certain mbox addresses of sdio_write.
> > > 
> > > Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> > > ---
> > >  drivers/net/wireless/ath/ath10k/sdio.c | 131
> > > ++++++++++++++++-----------------
> > >  1 file changed, 64 insertions(+), 67 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/ath/ath10k/sdio.c
> > > b/drivers/net/wireless/ath/ath10k/sdio.c
> > > index 77d4fa4..bb6fa67 100644
> > > --- a/drivers/net/wireless/ath/ath10k/sdio.c
> > > +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> > > @@ -36,6 +36,11 @@
> > > 
> > >  #define ATH10K_SDIO_DMA_BUF_SIZE	(32 * 1024)
> > > 
> > > +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
> > > +			    u32 len, bool incr);
> > > +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const
> > > void *buf,
> > > +			     u32 len, bool incr);
> > > +
> > 
> > As mentioned by Kalle, u32 needs to be size_t.
> Yes, the compiler I used is probably a step older and did not catch this.
> > 
> > >  /* inlined helper functions */
> > > 
> > >  /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
> > > @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
> > >  	struct sdio_func *func = ar_sdio->func;
> > >  	unsigned char byte, asyncintdelay = 2;
> > >  	int ret;
> > > +	u32 addr;
> > > 
> > >  	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
> > > 
> > > @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
> > >  		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
> > >  		 CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
> > > 
> > > -	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
> > > -					      CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> > > -					      byte);
> > > +	addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> > > +	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);
> > 
> > Not sure this part is needed.
> This is to overcome checkpatch warning. Too big a names for the function and
> macro
> not helping in there. Will have to move it as a separate patch.
> > 
> > >  	if (ret) {
> > >  		ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
> > >  		goto out;
> > > @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
> > > 
> > >  static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
> > >  {
> > > -	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> > > -	struct sdio_func *func = ar_sdio->func;
> > > +	__le32 *buf;
> > >  	int ret;
> > > 
> > > -	sdio_claim_host(func);
> > > +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> > > +	if (!buf)
> > > +		return -ENOMEM;
> > > 
> > > -	sdio_writel(func, val, addr, &ret);
> > > +	*buf = cpu_to_le32(val);
> > > +
> > > +	ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);
> > 
> > Shouldn't we use buf instead of val? buf seems pretty useless otherwise.
> Yes, thanks for pointing this out. will be corrected in v2.

Do you have any timeframe on the v2?

Regards,
Gary

Re: [PATCH 00/11] SDIO support for ath10k

[Gary Bisson]

Hi Alagu,

On Fri, Oct 06, 2017 at 01:16:13PM +0200, Gary Bisson wrote:
> Hi Alagu,
> 
> On Thu, Oct 05, 2017 at 10:54:26PM +0530, Alagu Sankar wrote:
> > Hi Gary,
> > 
> > On 05-10-2017 20:42, Gary Bisson wrote:
> > > Hi Alagu,
> > > 
> > > First of all, thank you for sharing your patches, this will be a very
> > > nice improvement to have SDIO QCA9377 working with ath10k.
> > > 
> > > I've tried your series with Nitrogen7 [1] platform which is supported in
> > > mainline already. It uses BD-SDMAC [2] which uses the same module as the
> > > SX-SDMAC.
> > > 
> > > Below are some questions/remarks I have after the testing.
> > > 
> > > On Sat, Sep 30, 2017 at 11:07:37PM +0530, silexcommon at gmail.com wrote:
> > > > From: Alagu Sankar <alagusankar at silex-india.com>
> > > > 
> > > > This patchset, generated against master-pending branch, enables a fully
> > > > functional SDIO interface driver for ath10k.  Patches have been verified on
> > > > QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access Point
> > > > and P2P modes.
> > > > 
> > > > The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
> > > Quick question on the firmware, is it the one from Kalle's repository?[3]
> > > 
> > > If so, where does this firmware comes from? Is 00061 the firmware
> > > version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 output:
> > > Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1
> > Yes, it is from
> > https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/untested.
> > I have also used custom firmware from QCA/Silex as used in qcacld-2.0 driver
> > without any issue. You need to use the firmware merger tool from
> > https://github.com/erstrom/linux-ath/wiki/Firmware to combine the
> > qwlan30.bin and otp30.bin to generate the firmware-sdio.bin.
> 
> Good to know, thanks. Maybe Kalle can tell us more about the firmware
> itself, what's the difference between the version 0.0.0.60 and 0.0.0.61?

Any update on this, is there a release notes for this 0.0.0.61 firmware?

> > > > with the board data from respective SDIO card vendors.
> > > About the board-sdio.bin, is it just a copy of your bdwlan30.bin?
> > Yes board-sdio.bin is a copy of bdwlan30.bin
> 
> Thanks for confirming it.
> 
> > > > Receive performance
> > > > matches the QCA reference driver when used with SDIO3.0 enabled platforms.
> > > > iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
> > > Nice performances. Unfortunately I don't get better than 30Mbits/s in TX
> > > and 50Mbits/s in RX:
> > > # iperf -c 192.168.1.1
> > > ------------------------------------------------------------
> > > Client connecting to 192.168.1.1, TCP port 5001
> > > TCP window size: 43.8 KByte (default)
> > > ------------------------------------------------------------
> > > [  3] local 192.168.1.115 port 41354 connected with 192.168.1.1 port
> > > 5001
> > > [ ID] Interval       Transfer     Bandwidth
> > > [  3]  0.0-10.0 sec  34.9 MBytes  29.2 Mbits/sec
> > > # iperf -s
> > > ------------------------------------------------------------
> > > Server listening on TCP port 5001
> > > TCP window size: 85.3 KByte (default)
> > > ------------------------------------------------------------
> > > [  4] local 192.168.1.115 port 5001 connected with 192.168.1.1 port
> > > 50646
> > > [ ID] Interval       Transfer     Bandwidth
> > > [  4]  0.0-10.0 sec  63.1 MBytes  52.7 Mbits/sec
> > > 
> > > Do you have any idea why? Note that qcacld-2.0 driver on that same
> > > platform (same OS) gives the performances you advertize (150Mbits/s).
> > For some reason, if I use the imx_v6_v7_defconfig as is, performance is very
> > poor. In fact, the firmware download itself will take about 6 seconds. This
> > can also be seen when you up/down the wlan0 interface. For the i.MX6 SoloX
> > board, I used the configuration of 4.1.15 as provided by the BSP from
> > NXP/Freescale.
> 
> Do you mean that you've used 4.1.15 kernel or just the 4.1.15
> configuration on latest 4.14?

As a FYI, I've tried your ath10k patches (along with a few backported
patches from Erik) on the NXP-fork of 4.9 kernel [1].

I confirm that it provides pretty decent performances:
- ath10k: 110Mbit/s
- qcacld-2.0: 125Mbit/s

Here are some details about the setup:
- TPLink AC router
- Nitrogen7 (i.MX7) with BD-SDMAC
- Kernel 4.9.68 (NXP fork [1])
- FW 0.0.0.61 from ath10k-firmware repo

It is definitely a nice alternative to qcacld driver. When do you plan
on sending a v2 out?

Regards,
Gary

[1] https://github.com/boundarydevices/linux-imx6/commits/test-ath10k

Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[Kalle Valo]

silexcommon@gmail.com writes:

> From: Alagu Sankar <alagusankar@silex-india.com>
>
> Some SD host controllers still need bounce buffers for SDIO data
> transfers. While the transfers worked fine on x86 platforms,
> this is found to be required for i.MX6 based systems.
>
> Changes are similar to and derived from the ath6kl sdio driver.
>
> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>

Why is the bounce buffer needed exactly, what are the symptoms etc? To
me this sounds like an ugly workaround for a SDIO controller driver bug.

--=20
Kalle Valo=

Re: [PATCH 00/11] SDIO support for ath10k

[Kalle Valo]

Gary Bisson <gary.bisson@boundarydevices.com> writes:

> On Fri, Oct 06, 2017 at 01:16:13PM +0200, Gary Bisson wrote:
>> On Thu, Oct 05, 2017 at 10:54:26PM +0530, Alagu Sankar wrote:
>> > On 05-10-2017 20:42, Gary Bisson wrote:
>> > >=20
>> > > First of all, thank you for sharing your patches, this will be a ver=
y
>> > > nice improvement to have SDIO QCA9377 working with ath10k.
>> > >=20
>> > > I've tried your series with Nitrogen7 [1] platform which is supporte=
d in
>> > > mainline already. It uses BD-SDMAC [2] which uses the same module as=
 the
>> > > SX-SDMAC.
>> > >=20
>> > > Below are some questions/remarks I have after the testing.
>> > >=20
>> > > Quick question on the firmware, is it the one from Kalle's repositor=
y?[3]
>> > >=20
>> > > If so, where does this firmware comes from? Is 00061 the firmware
>> > > version? So far I've only seen up to v0.0.0.60, see qcacld-2.0 outpu=
t:
>> > > Host SW:4.5.20.037, FW:0.0.0.60, HW:QCA9377_REV1_1
>> >
>> > Yes, it is from
>> > https://github.com/kvalo/ath10k-firmware/tree/master/QCA9377/hw1.0/unt=
ested.
>> > I have also used custom firmware from QCA/Silex as used in qcacld-2.0 =
driver
>> > without any issue. You need to use the firmware merger tool from
>> > https://github.com/erstrom/linux-ath/wiki/Firmware to combine the
>> > qwlan30.bin and otp30.bin to generate the firmware-sdio.bin.
>>=20
>> Good to know, thanks. Maybe Kalle can tell us more about the firmware
>> itself, what's the difference between the version 0.0.0.60 and 0.0.0.61?
>
> Any update on this, is there a release notes for this 0.0.0.61 firmware?

Unfortunately I don't have any changelogs for the firmware releases.

> As a FYI, I've tried your ath10k patches (along with a few backported
> patches from Erik) on the NXP-fork of 4.9 kernel [1].
>
> I confirm that it provides pretty decent performances:
> - ath10k: 110Mbit/s
> - qcacld-2.0: 125Mbit/s
>
> Here are some details about the setup:
> - TPLink AC router
> - Nitrogen7 (i.MX7) with BD-SDMAC
> - Kernel 4.9.68 (NXP fork [1])
> - FW 0.0.0.61 from ath10k-firmware repo

Nice, thanks for the report. This is always helpful.

--=20
Kalle Valo=

Re: [PATCH 00/11] SDIO support for ath10k

[Kalle Valo]

silexcommon@gmail.com writes:

> From: Alagu Sankar <alagusankar@silex-india.com>
>
> This patchset, generated against master-pending branch, enables a fully
> functional SDIO interface driver for ath10k.  Patches have been verified =
on
> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station, Access=
 Point
> and P2P modes.
>
> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
> with the board data from respective SDIO card vendors. Receive performanc=
e
> matches the QCA reference driver when used with SDIO3.0 enabled platforms=
.
> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
>
> This patchset differs from the previous high latency patches, specific to=
 SDIO.
> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This instruc=
ts the
> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets. Witho=
ut
> this flag, the management frames are not sent out by the firmware. Possib=
ility
> of management frames being sent via WMI and data frames through the reduc=
ed Tx
> completion needs to be probed further.
>
> Further improvements can be done on the transmit path by implementing pac=
ket
> bundle. Scatter Gather is another area of improvement for both Transmit a=
nd
> Receive, but may not work on all platforms
>
> Known issues: Surprise removal of the card, when the device is in connect=
ed
> state, delays sdio function remove due to delayed WMI command failures.
> Existing ath10k framework can not differentiate between a kernel module
> removal and the surprise removal of teh card.
>
> Alagu Sankar (11):
>   ath10k_sdio: sdio htt data transfer fixes
>   ath10k_sdio: wb396 reference card fix
>   ath10k_sdio: DMA bounce buffers for read write
>   ath10k_sdio: reduce transmit msdu count
>   ath10k_sdio: use clean packet headers
>   ath10k_sdio: high latency fixes for beacon buffer
>   ath10k_sdio: fix rssi indication
>   ath10k_sdio: common read write
>   ath10k_sdio: virtual scatter gather for receive
>   ath10k_sdio: enable firmware crash dump
>   ath10k_sdio: hif start once addition

Sorry, I run out of time to review this in detail. To make the review
easier I recommend to split this patchset into two sets, first set
containing only the bare essential to get basic functionality working
(for example ping working on x86) and the second set containing all the
optimisations (the bounce buffer stuff etc).

And try to make the first set as small as possible so that we can get it
faster applied.

--=20
Kalle Valo=

Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[Alagu Sankar]

On 2017-12-22 21:38, Kalle Valo wrote:
> silexcommon@gmail.com writes:
> 
>> From: Alagu Sankar <alagusankar@silex-india.com>
>> 
>> Some SD host controllers still need bounce buffers for SDIO data
>> transfers. While the transfers worked fine on x86 platforms,
>> this is found to be required for i.MX6 based systems.
>> 
>> Changes are similar to and derived from the ath6kl sdio driver.
>> 
>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
> 
> Why is the bounce buffer needed exactly, what are the symptoms etc? To
> me this sounds like an ugly workaround for a SDIO controller driver 
> bug.

We faced problems with i.MX6. The authentication frame sent by the 
driver never reached the air. The host driver accepted the buffer, but 
did not send out the packet to the sdio module. No errors reported 
anywhere, but the buffer is not accepted due to alignment. The same 
driver however works fine without bounce buffer on x86 platform with 
stdhci drivers. To make it compliant with all host controllers, we 
introduced the bounce buffers, similar to what was done in ath6kl_sdio 
drivers.

Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[Adrian Chadd]

Hi,

I think Kalle is pointing out that maybe it's the SDHCI driver
responsibility to do the bounce buffering?



-adrian

Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[Arend van Spriel]

On 12/25/2017 1:26 PM, Alagu Sankar wrote:
> On 2017-12-22 21:38, Kalle Valo wrote:
>> silexcommon@gmail.com writes:
>>
>>> From: Alagu Sankar <alagusankar@silex-india.com>
>>>
>>> Some SD host controllers still need bounce buffers for SDIO data
>>> transfers. While the transfers worked fine on x86 platforms,
>>> this is found to be required for i.MX6 based systems.
>>>
>>> Changes are similar to and derived from the ath6kl sdio driver.
>>>
>>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>>
>> Why is the bounce buffer needed exactly, what are the symptoms etc? To
>> me this sounds like an ugly workaround for a SDIO controller driver bug.
>
> We faced problems with i.MX6. The authentication frame sent by the
> driver never reached the air. The host driver accepted the buffer, but
> did not send out the packet to the sdio module. No errors reported
> anywhere, but the buffer is not accepted due to alignment. The same
> driver however works fine without bounce buffer on x86 platform with
> stdhci drivers. To make it compliant with all host controllers, we
> introduced the bounce buffers, similar to what was done in ath6kl_sdio
> drivers.

As mentioned by Adrian the comment from Kalle is that you are solving an 
issue caused by the sdio host controller. Although strictly speaking it 
may not be a driver bug, but a requirement of the host controller 
hardware. Either way it seems the obvious place to solve this is in the 
sdio host controller driver to which the issue applies. Or make it a 
generic quirk which can be enabled for sdio host controller drivers that 
need it. However, there may reasons to do it in the networking driver. 
For instance, the buffer you want to transfer might be the data buffer 
of an sk_buff you got from the networking stack and you want to have a 
zero-copy solution towards the wireless device.

Your solution checks for 4-byte alignment which is a requirement for 
ADMA as per SDIO spec. However, I have come across host controllers 
which have different alignment requirements. Also when 
CONFIG_ARCH_DMA_ADDR_T_64BIT is enabled the alignment changes from 4 to 
8 bytes. So it seems you are solving a specific case you have come 
across, but you may want to design for more flexibility.

Hope this helps.

Regards,
Arend

Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[Adrian Chadd]

[top post for emphasis]

Arend is right. You won't be the only driver which has issues with a
controller that doesn't handle non-aligned data payloads. Please push
it into the stack or the controller side, but not in the driver side.
That'll be a forever game of whack-a-mole.


-adrian

(I'm living this dream right now and it's unfun)



On 27 December 2017 at 10:49, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 12/25/2017 1:26 PM, Alagu Sankar wrote:
>>
>> On 2017-12-22 21:38, Kalle Valo wrote:
>>>
>>> silexcommon@gmail.com writes:
>>>
>>>> From: Alagu Sankar <alagusankar@silex-india.com>
>>>>
>>>> Some SD host controllers still need bounce buffers for SDIO data
>>>> transfers. While the transfers worked fine on x86 platforms,
>>>> this is found to be required for i.MX6 based systems.
>>>>
>>>> Changes are similar to and derived from the ath6kl sdio driver.
>>>>
>>>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>>>
>>>
>>> Why is the bounce buffer needed exactly, what are the symptoms etc? To
>>> me this sounds like an ugly workaround for a SDIO controller driver bug.
>>
>>
>> We faced problems with i.MX6. The authentication frame sent by the
>> driver never reached the air. The host driver accepted the buffer, but
>> did not send out the packet to the sdio module. No errors reported
>> anywhere, but the buffer is not accepted due to alignment. The same
>> driver however works fine without bounce buffer on x86 platform with
>> stdhci drivers. To make it compliant with all host controllers, we
>> introduced the bounce buffers, similar to what was done in ath6kl_sdio
>> drivers.
>
>
> As mentioned by Adrian the comment from Kalle is that you are solving an
> issue caused by the sdio host controller. Although strictly speaking it may
> not be a driver bug, but a requirement of the host controller hardware.
> Either way it seems the obvious place to solve this is in the sdio host
> controller driver to which the issue applies. Or make it a generic quirk
> which can be enabled for sdio host controller drivers that need it. However,
> there may reasons to do it in the networking driver. For instance, the
> buffer you want to transfer might be the data buffer of an sk_buff you got
> from the networking stack and you want to have a zero-copy solution towards
> the wireless device.
>
> Your solution checks for 4-byte alignment which is a requirement for ADMA as
> per SDIO spec. However, I have come across host controllers which have
> different alignment requirements. Also when CONFIG_ARCH_DMA_ADDR_T_64BIT is
> enabled the alignment changes from 4 to 8 bytes. So it seems you are solving
> a specific case you have come across, but you may want to design for more
> flexibility.
>
> Hope this helps.
>
> Regards,
> Arend

Re: [PATCH 03/11] ath10k_sdio: DMA bounce buffers for read write

[Kalle Valo]

Alagu Sankar <alagusankar@silex-india.com> writes:

> On 2017-12-22 21:38, Kalle Valo wrote:
>> silexcommon@gmail.com writes:
>>
>>> From: Alagu Sankar <alagusankar@silex-india.com>
>>>
>>> Some SD host controllers still need bounce buffers for SDIO data
>>> transfers. While the transfers worked fine on x86 platforms,
>>> this is found to be required for i.MX6 based systems.
>>>
>>> Changes are similar to and derived from the ath6kl sdio driver.
>>>
>>> Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
>>
>> Why is the bounce buffer needed exactly, what are the symptoms etc? To
>> me this sounds like an ugly workaround for a SDIO controller driver
>> bug.
>
> We faced problems with i.MX6. The authentication frame sent by the
> driver never reached the air. The host driver accepted the buffer, but
> did not send out the packet to the sdio module. No errors reported
> anywhere, but the buffer is not accepted due to alignment.

So what kind of alignment works with i.MX6 and how are the packets
aligned by ath10k? Of course, this might be still a bug in ath10k but
most likely it's elsewhere and should be properly investigated. There
must be a much better approach to handle this problem.

> The same driver however works fine without bounce buffer on x86
> platform with stdhci drivers. To make it compliant with all host
> controllers, we introduced the bounce buffers, similar to what was
> done in ath6kl_sdio drivers.

That bounce buffer was horrible in ath6kl and with the bounce buffer you
are forcing all working platforms to suffer from a copy of every packet.
And ath10k is getting so complex that we really need to keep the code as
simple as possible to keep it maintainable.

--=20
Kalle Valo=