[RFC v2 00/11] ath10k sdio support

[RFC v2 00/11] ath10k sdio support

[Erik Stromdahl]

Changes since last version:

The BMI patch (no. 6 in the previous version) has been removed since
it is not used by the last (major) sdio patch.

The QCA65XX enum patch (no. 10 in the previous version) was also
removed since it was considered not necessary.

Added a new patch (no. 3) that removes two unused members in
struct ath10k_htc_ep. These removals makes it possible to connect
the HTC control endpoint before wait target (just as ath6kl does).

Updated commit comments for a few patches.

Other updates according to the comments from Michal Kazior and
Kalle Valo.

The new version was built and tested against:
tag: ath-201611151509


*Original description*

This patch series adds sdio support to ath10k.

Overview:
A new HIF layer: sdio/mailbox.
The current HIF ops are unaltered even though some ops
are not applicable for sdio.

The HTC layer has only suffered minor modifications:
- A few new functions for handling the mailbox specific
  RX trailers (lookahead reports)
- Some minor refactorization of the existing code
  (patches 3 and 4)

This is not included in this patch series:

- HTT High latency RX and TX support
- Full integration in core.c

The following basic tests have been made so far:

BMI fw load and firmware startup (all the steps in ath10k_core_start).
This means:

- HTT service connect
- WMI control service connect
- WMI unified init

The above mentioned bullets where verified with a QCA6584 chipset.

I have not been able to test the patch series together with
ath10k pcie hardware, but I will do so as soon as I can get
my hands on some hardware.

The patches have been built and tested against the ath tree:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
branch/tag: master/ath-201611090123

Erik Stromdahl (11):
  ath10k: htc: made static function public
  ath10k: htc: rx trailer lookahead support
  ath10k: htc: Removal of unused struct members
  ath10k: htc: Changed order of wait target and ep connect
  ath10k: htc: refactorization
  ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB
  ath10k: Added SDIO dbg masks
  ath10k: Added ATH10K_BUS_SDIO enum
  ath10k: Mailbox address definitions
  ath10k: Added more host_interest members
  ath10k: Added sdio support

 drivers/net/wireless/ath/ath10k/Kconfig     |    6 +
 drivers/net/wireless/ath/ath10k/Makefile    |    3 +
 drivers/net/wireless/ath/ath10k/core.h      |    3 +
 drivers/net/wireless/ath/ath10k/debug.h     |    2 +
 drivers/net/wireless/ath/ath10k/htc.c       |  214 ++-
 drivers/net/wireless/ath/ath10k/htc.h       |   38 +-
 drivers/net/wireless/ath/ath10k/hw.h        |   53 +
 drivers/net/wireless/ath/ath10k/sdio.c      | 1860 +++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/sdio.h      |  276 ++++
 drivers/net/wireless/ath/ath10k/targaddrs.h |   24 +
 10 files changed, 2407 insertions(+), 72 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/sdio.c
 create mode 100644 drivers/net/wireless/ath/ath10k/sdio.h

-- 
1.7.9.5

[RFC v2 01/11] ath10k: htc: made static function public

[Erik Stromdahl]

Changed ath10k_htc_notify_tx_completion and
ath10k_htc_process_trailer from static to non static.

These functions are needed by SDIO/mbox.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htc.c |   14 ++++++++------
 drivers/net/wireless/ath/ath10k/htc.h |    6 ++++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 175aae3..26b1e15 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -57,8 +57,8 @@ static inline void ath10k_htc_restore_tx_skb(struct ath10k_htc *htc,
 	skb_pull(skb, sizeof(struct ath10k_htc_hdr));
 }
 
-static void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
-					    struct sk_buff *skb)
+void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
+				     struct sk_buff *skb)
 {
 	struct ath10k *ar = ep->htc->ar;
 
@@ -75,6 +75,7 @@ static void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
 
 	ep->ep_ops.ep_tx_complete(ep->htc->ar, skb);
 }
+EXPORT_SYMBOL(ath10k_htc_notify_tx_completion);
 
 static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep,
 				      struct sk_buff *skb)
@@ -227,10 +228,10 @@ void ath10k_htc_tx_completion_handler(struct ath10k *ar, struct sk_buff *skb)
 	spin_unlock_bh(&htc->tx_lock);
 }
 
-static int ath10k_htc_process_trailer(struct ath10k_htc *htc,
-				      u8 *buffer,
-				      int length,
-				      enum ath10k_htc_ep_id src_eid)
+int ath10k_htc_process_trailer(struct ath10k_htc *htc,
+			       u8 *buffer,
+			       int length,
+			       enum ath10k_htc_ep_id src_eid)
 {
 	struct ath10k *ar = htc->ar;
 	int status = 0;
@@ -291,6 +292,7 @@ static int ath10k_htc_process_trailer(struct ath10k_htc *htc,
 
 	return status;
 }
+EXPORT_SYMBOL(ath10k_htc_process_trailer);
 
 void ath10k_htc_rx_completion_handler(struct ath10k *ar, struct sk_buff *skb)
 {
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 0c55cd9..858e19f 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -354,5 +354,11 @@ int ath10k_htc_send(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid,
 struct sk_buff *ath10k_htc_alloc_skb(struct ath10k *ar, int size);
 void ath10k_htc_tx_completion_handler(struct ath10k *ar, struct sk_buff *skb);
 void ath10k_htc_rx_completion_handler(struct ath10k *ar, struct sk_buff *skb);
+void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
+				     struct sk_buff *skb);
+int ath10k_htc_process_trailer(struct ath10k_htc *htc,
+			       u8 *buffer,
+			       int length,
+			       enum ath10k_htc_ep_id src_eid);
 
 #endif
-- 
1.7.9.5

[RFC v2 02/11] ath10k: htc: rx trailer lookahead support

[Erik Stromdahl]

The RX trailer parsing is now capable of parsing lookahead reports.
A lookahead contains the first 4 bytes of the next HTC message
(that will be read in the next SDIO read operation).
Lookaheads are used by the SDIO/mbox HIF layer to determine if
the next message is part of a bundle, which endpoint it belongs
to and how long it is.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htc.c |   91 ++++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/htc.h |   30 +++++++++--
 2 files changed, 116 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 26b1e15..957e828 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -228,10 +228,74 @@ void ath10k_htc_tx_completion_handler(struct ath10k *ar, struct sk_buff *skb)
 	spin_unlock_bh(&htc->tx_lock);
 }
 
+static int
+ath10k_htc_process_lookahead(struct ath10k_htc *htc,
+			     const struct ath10k_htc_lookahead_report *report,
+			     int len,
+			     enum ath10k_htc_ep_id eid,
+			     void *next_lookaheads,
+			     int *next_lookaheads_len)
+{
+	struct ath10k *ar = htc->ar;
+
+	/* Invalid lookahead flags are actually transmitted by
+	 * the target in the HTC control message.
+	 * Since this will happen at every boot we silently ignore
+	 * the lookahead in this case
+	 */
+	if (report->pre_valid != ((~report->post_valid) & 0xFF))
+		return 0;
+
+	if (next_lookaheads && next_lookaheads_len) {
+		ath10k_dbg(ar, ATH10K_DBG_HTC,
+			   "htc rx lookahead found pre_valid 0x%x post_valid 0x%x\n",
+			   report->pre_valid, report->post_valid);
+
+		/* look ahead bytes are valid, copy them over */
+		memcpy((u8 *)next_lookaheads, report->lookahead, 4);
+
+		*next_lookaheads_len = 1;
+	}
+
+	return 0;
+}
+
+static int
+ath10k_htc_process_lookahead_bundle(struct ath10k_htc *htc,
+				    const struct ath10k_htc_lookahead_report_bundle *report,
+				    int len,
+				    enum ath10k_htc_ep_id eid,
+				    void *next_lookaheads,
+				    int *next_lookaheads_len)
+{
+	int bundle_cnt = len / sizeof(*report);
+
+	if (!bundle_cnt || (bundle_cnt > HTC_HOST_MAX_MSG_PER_BUNDLE)) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	if (next_lookaheads && next_lookaheads_len) {
+		int i;
+
+		for (i = 0; i < bundle_cnt; i++) {
+			memcpy(((u8 *)next_lookaheads) + 4 * i,
+			       report->lookahead, 4);
+			report++;
+		}
+
+		*next_lookaheads_len = bundle_cnt;
+	}
+
+	return 0;
+}
+
 int ath10k_htc_process_trailer(struct ath10k_htc *htc,
 			       u8 *buffer,
 			       int length,
-			       enum ath10k_htc_ep_id src_eid)
+			       enum ath10k_htc_ep_id src_eid,
+			       void *next_lookaheads,
+			       int *next_lookaheads_len)
 {
 	struct ath10k *ar = htc->ar;
 	int status = 0;
@@ -272,6 +336,28 @@ int ath10k_htc_process_trailer(struct ath10k_htc *htc,
 							 record->hdr.len,
 							 src_eid);
 			break;
+		case ATH10K_HTC_RECORD_LOOKAHEAD:
+			len = sizeof(struct ath10k_htc_lookahead_report);
+			if (record->hdr.len < len) {
+				ath10k_warn(ar, "Lookahead report too long\n");
+				status = -EINVAL;
+				break;
+			}
+			status = ath10k_htc_process_lookahead(htc,
+							      record->lookahead_report,
+							      record->hdr.len,
+							      src_eid,
+							      next_lookaheads,
+							      next_lookaheads_len);
+			break;
+		case ATH10K_HTC_RECORD_LOOKAHEAD_BUNDLE:
+			status = ath10k_htc_process_lookahead_bundle(htc,
+								     record->lookahead_bundle,
+								     record->hdr.len,
+								     src_eid,
+								     next_lookaheads,
+								     next_lookaheads_len);
+			break;
 		default:
 			ath10k_warn(ar, "Unhandled record: id:%d length:%d\n",
 				    record->hdr.id, record->hdr.len);
@@ -359,7 +445,8 @@ void ath10k_htc_rx_completion_handler(struct ath10k *ar, struct sk_buff *skb)
 		trailer += payload_len;
 		trailer -= trailer_len;
 		status = ath10k_htc_process_trailer(htc, trailer,
-						    trailer_len, hdr->eid);
+						    trailer_len, hdr->eid,
+						    NULL, NULL);
 		if (status)
 			goto out;
 
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 858e19f..b0f9cf3 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -50,6 +50,8 @@
  * 4-byte aligned.
  */
 
+#define HTC_HOST_MAX_MSG_PER_BUNDLE        8
+
 enum ath10k_htc_tx_flags {
 	ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE = 0x01,
 	ATH10K_HTC_FLAG_SEND_BUNDLE        = 0x02
@@ -174,8 +176,10 @@ struct ath10k_htc_msg {
 } __packed __aligned(4);
 
 enum ath10k_ath10k_htc_record_id {
-	ATH10K_HTC_RECORD_NULL    = 0,
-	ATH10K_HTC_RECORD_CREDITS = 1
+	ATH10K_HTC_RECORD_NULL             = 0,
+	ATH10K_HTC_RECORD_CREDITS          = 1,
+	ATH10K_HTC_RECORD_LOOKAHEAD        = 2,
+	ATH10K_HTC_RECORD_LOOKAHEAD_BUNDLE = 3,
 };
 
 struct ath10k_ath10k_htc_record_hdr {
@@ -192,10 +196,28 @@ struct ath10k_htc_credit_report {
 	u8 pad1;
 } __packed;
 
+struct ath10k_htc_lookahead_report {
+	u8 pre_valid;
+	u8 pad0;
+	u8 pad1;
+	u8 pad2;
+	u8 lookahead[4];
+	u8 post_valid;
+	u8 pad3;
+	u8 pad4;
+	u8 pad5;
+} __packed;
+
+struct ath10k_htc_lookahead_report_bundle {
+	u8 lookahead[4];
+} __packed;
+
 struct ath10k_htc_record {
 	struct ath10k_ath10k_htc_record_hdr hdr;
 	union {
 		struct ath10k_htc_credit_report credit_report[0];
+		struct ath10k_htc_lookahead_report lookahead_report[0];
+		struct ath10k_htc_lookahead_report_bundle lookahead_bundle[0];
 		u8 pauload[0];
 	};
 } __packed __aligned(4);
@@ -359,6 +381,8 @@ void ath10k_htc_notify_tx_completion(struct ath10k_htc_ep *ep,
 int ath10k_htc_process_trailer(struct ath10k_htc *htc,
 			       u8 *buffer,
 			       int length,
-			       enum ath10k_htc_ep_id src_eid);
+			       enum ath10k_htc_ep_id src_eid,
+			       void *next_lookaheads,
+			       int *next_lookaheads_len);
 
 #endif
-- 
1.7.9.5

[RFC v2 03/11] ath10k: htc: Removal of unused struct members

[Erik Stromdahl]

Removed tx_credits_per_max_message and tx_credit_size
from struct ath10k_htc_ep since they are not used
anywhere in the code.

They are just written, never read.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htc.c |    6 ------
 drivers/net/wireless/ath/ath10k/htc.h |    2 --
 2 files changed, 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 957e828..79d44de 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -815,12 +815,6 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
 	ep->max_tx_queue_depth = conn_req->max_send_queue_depth;
 	ep->max_ep_message_len = __le16_to_cpu(resp_msg->max_msg_size);
 	ep->tx_credits = tx_alloc;
-	ep->tx_credit_size = htc->target_credit_size;
-	ep->tx_credits_per_max_message = ep->max_ep_message_len /
-					 htc->target_credit_size;
-
-	if (ep->max_ep_message_len % htc->target_credit_size)
-		ep->tx_credits_per_max_message++;
 
 	/* copy all the callbacks */
 	ep->ep_ops = conn_req->ep_ops;
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index b0f9cf3..f94b25a 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -336,8 +336,6 @@ struct ath10k_htc_ep {
 
 	u8 seq_no; /* for debugging */
 	int tx_credits;
-	int tx_credit_size;
-	int tx_credits_per_max_message;
 	bool tx_credit_flow_enabled;
 };
 
-- 
1.7.9.5

[RFC v2 04/11] ath10k: htc: Changed order of wait target and ep connect

[Erik Stromdahl]

This patch changes the order in which the driver waits for the
target to become ready and the service connect of the HTC
control service.

The HTC control service is connected before the driver starts
waiting for the HTC ready message.

The HTC ready message contains the total number of transmit
credits the driver can distribute between endpoints.
Since the HTC control service does not use any flow control
it is not necessary to wait for the ready message before
connecting the service.
There will be no credits assigned to this service anyway.
Besides, connecting the HTC control service does not yield
any bus traffic at all.

The ready message will always be transmitted on endpoint 0
(which is always assigned to the HTC control service) so it
makes more sense if HTC control has been connected before the
ready message is received.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htc.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 79d44de..6ff5837 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -606,6 +606,22 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
 	u16 credit_count;
 	u16 credit_size;
 
+	/* setup our pseudo HTC control endpoint connection */
+	memset(&conn_req, 0, sizeof(conn_req));
+	memset(&conn_resp, 0, sizeof(conn_resp));
+	conn_req.ep_ops.ep_tx_complete = ath10k_htc_control_tx_complete;
+	conn_req.ep_ops.ep_rx_complete = ath10k_htc_control_rx_complete;
+	conn_req.max_send_queue_depth = ATH10K_NUM_CONTROL_TX_BUFFERS;
+	conn_req.service_id = ATH10K_HTC_SVC_ID_RSVD_CTRL;
+
+	/* connect fake service */
+	status = ath10k_htc_connect_service(htc, &conn_req, &conn_resp);
+	if (status) {
+		ath10k_err(ar, "could not connect to htc service (%d)\n",
+			   status);
+		return status;
+	}
+
 	time_left = wait_for_completion_timeout(&htc->ctl_resp,
 						ATH10K_HTC_WAIT_TIMEOUT_HZ);
 	if (!time_left) {
@@ -665,22 +681,6 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
 
 	ath10k_htc_setup_target_buffer_assignments(htc);
 
-	/* setup our pseudo HTC control endpoint connection */
-	memset(&conn_req, 0, sizeof(conn_req));
-	memset(&conn_resp, 0, sizeof(conn_resp));
-	conn_req.ep_ops.ep_tx_complete = ath10k_htc_control_tx_complete;
-	conn_req.ep_ops.ep_rx_complete = ath10k_htc_control_rx_complete;
-	conn_req.max_send_queue_depth = ATH10K_NUM_CONTROL_TX_BUFFERS;
-	conn_req.service_id = ATH10K_HTC_SVC_ID_RSVD_CTRL;
-
-	/* connect fake service */
-	status = ath10k_htc_connect_service(htc, &conn_req, &conn_resp);
-	if (status) {
-		ath10k_err(ar, "could not connect to htc service (%d)\n",
-			   status);
-		return status;
-	}
-
 	return 0;
 }
 
-- 
1.7.9.5

[RFC v2 05/11] ath10k: htc: refactorization

[Erik Stromdahl]

Code refactorization:

Moved the code for ep 0 in ath10k_htc_rx_completion_handler
to ath10k_htc_control_rx_complete.

This eases the implementation of SDIO/mbox significantly since
the ep_rx_complete cb is invoked directly from the SDIO/mbox
hif layer.

Since the ath10k_htc_control_rx_complete already is present
(only containing a warning message) there is no reason for not
using it (instead of having a special case for ep 0 in
ath10k_htc_rx_completion_handler).

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htc.c |   73 +++++++++++++++------------------
 1 file changed, 34 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 6ff5837..f4ffeef 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -457,42 +457,6 @@ void ath10k_htc_rx_completion_handler(struct ath10k *ar, struct sk_buff *skb)
 		/* zero length packet with trailer data, just drop these */
 		goto out;
 
-	if (eid == ATH10K_HTC_EP_0) {
-		struct ath10k_htc_msg *msg = (struct ath10k_htc_msg *)skb->data;
-
-		switch (__le16_to_cpu(msg->hdr.message_id)) {
-		case ATH10K_HTC_MSG_READY_ID:
-		case ATH10K_HTC_MSG_CONNECT_SERVICE_RESP_ID:
-			/* handle HTC control message */
-			if (completion_done(&htc->ctl_resp)) {
-				/*
-				 * this is a fatal error, target should not be
-				 * sending unsolicited messages on the ep 0
-				 */
-				ath10k_warn(ar, "HTC rx ctrl still processing\n");
-				complete(&htc->ctl_resp);
-				goto out;
-			}
-
-			htc->control_resp_len =
-				min_t(int, skb->len,
-				      ATH10K_HTC_MAX_CTRL_MSG_LEN);
-
-			memcpy(htc->control_resp_buffer, skb->data,
-			       htc->control_resp_len);
-
-			complete(&htc->ctl_resp);
-			break;
-		case ATH10K_HTC_MSG_SEND_SUSPEND_COMPLETE:
-			htc->htc_ops.target_send_suspend_complete(ar);
-			break;
-		default:
-			ath10k_warn(ar, "ignoring unsolicited htc ep0 event\n");
-			break;
-		}
-		goto out;
-	}
-
 	ath10k_dbg(ar, ATH10K_DBG_HTC, "htc rx completion ep %d skb %pK\n",
 		   eid, skb);
 	ep->ep_ops.ep_rx_complete(ar, skb);
@@ -507,9 +471,40 @@ void ath10k_htc_rx_completion_handler(struct ath10k *ar, struct sk_buff *skb)
 static void ath10k_htc_control_rx_complete(struct ath10k *ar,
 					   struct sk_buff *skb)
 {
-	/* This is unexpected. FW is not supposed to send regular rx on this
-	 * endpoint. */
-	ath10k_warn(ar, "unexpected htc rx\n");
+	struct ath10k_htc *htc = &ar->htc;
+	struct ath10k_htc_msg *msg = (struct ath10k_htc_msg *)skb->data;
+
+	switch (__le16_to_cpu(msg->hdr.message_id)) {
+	case ATH10K_HTC_MSG_READY_ID:
+	case ATH10K_HTC_MSG_CONNECT_SERVICE_RESP_ID:
+		/* handle HTC control message */
+		if (completion_done(&htc->ctl_resp)) {
+			/* this is a fatal error, target should not be
+			 * sending unsolicited messages on the ep 0
+			 */
+			ath10k_warn(ar, "HTC rx ctrl still processing\n");
+			complete(&htc->ctl_resp);
+			goto out;
+		}
+
+		htc->control_resp_len =
+			min_t(int, skb->len,
+			      ATH10K_HTC_MAX_CTRL_MSG_LEN);
+
+		memcpy(htc->control_resp_buffer, skb->data,
+		       htc->control_resp_len);
+
+		complete(&htc->ctl_resp);
+		break;
+	case ATH10K_HTC_MSG_SEND_SUSPEND_COMPLETE:
+		htc->htc_ops.target_send_suspend_complete(ar);
+		break;
+	default:
+		ath10k_warn(ar, "ignoring unsolicited htc ep0 event\n");
+		break;
+	}
+
+out:
 	kfree_skb(skb);
 }
 
-- 
1.7.9.5

[RFC v2 06/11] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB

[Erik Stromdahl]

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/htc.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index f94b25a..2963694 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
 	ATH10K_HTC_FLAG_BUNDLE_MASK     = 0xF0
 };
 
+#define ATH10K_HTC_FLAG_BUNDLE_LSB         4
+
 struct ath10k_htc_hdr {
 	u8 eid; /* @enum ath10k_htc_ep_id */
 	u8 flags; /* @enum ath10k_htc_tx_flags, ath10k_htc_rx_flags */
-- 
1.7.9.5

[RFC v2 07/11] ath10k: Added SDIO dbg masks

[Erik Stromdahl]

Debug masks for SDIO HIF layer.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/debug.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 335512b..d35263c 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -38,6 +38,8 @@ enum ath10k_debug_mask {
 	ATH10K_DBG_WMI_PRINT	= 0x00002000,
 	ATH10K_DBG_PCI_PS	= 0x00004000,
 	ATH10K_DBG_AHB		= 0x00008000,
+	ATH10K_DBG_SDIO		= 0x00010000,
+	ATH10K_DBG_SDIO_DUMP	= 0x00020000,
 	ATH10K_DBG_ANY		= 0xffffffff,
 };
 
-- 
1.7.9.5

[RFC v2 08/11] ath10k: Added ATH10K_BUS_SDIO enum

[Erik Stromdahl]

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/core.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index e8decfa..be6cd55 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -74,6 +74,7 @@
 enum ath10k_bus {
 	ATH10K_BUS_PCI,
 	ATH10K_BUS_AHB,
+	ATH10K_BUS_SDIO,
 };
 
 static inline const char *ath10k_bus_str(enum ath10k_bus bus)
@@ -83,6 +84,8 @@ static inline const char *ath10k_bus_str(enum ath10k_bus bus)
 		return "pci";
 	case ATH10K_BUS_AHB:
 		return "ahb";
+	case ATH10K_BUS_SDIO:
+		return "sdio";
 	}
 
 	return "unknown";
-- 
1.7.9.5

[RFC v2 09/11] ath10k: Mailbox address definitions

[Erik Stromdahl]

Address definitions for SDIO/mbox based chipsets.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/hw.h |   53 ++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 883547f..46142e9 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -814,6 +814,59 @@ struct ath10k_hw_ops {
 #define QCA9887_EEPROM_ADDR_LO_MASK		0x00ff0000
 #define QCA9887_EEPROM_ADDR_LO_LSB		16
 
+#define MBOX_RESET_CONTROL_ADDRESS		0x00000000
+#define MBOX_HOST_INT_STATUS_ADDRESS		0x00000800
+#define MBOX_HOST_INT_STATUS_ERROR_LSB		7
+#define MBOX_HOST_INT_STATUS_ERROR_MASK		0x00000080
+#define MBOX_HOST_INT_STATUS_CPU_LSB		6
+#define MBOX_HOST_INT_STATUS_CPU_MASK		0x00000040
+#define MBOX_HOST_INT_STATUS_COUNTER_LSB	4
+#define MBOX_HOST_INT_STATUS_COUNTER_MASK	0x00000010
+#define MBOX_CPU_INT_STATUS_ADDRESS		0x00000801
+#define MBOX_ERROR_INT_STATUS_ADDRESS		0x00000802
+#define MBOX_ERROR_INT_STATUS_WAKEUP_LSB	2
+#define MBOX_ERROR_INT_STATUS_WAKEUP_MASK	0x00000004
+#define MBOX_ERROR_INT_STATUS_RX_UNDERFLOW_LSB	1
+#define MBOX_ERROR_INT_STATUS_RX_UNDERFLOW_MASK	0x00000002
+#define MBOX_ERROR_INT_STATUS_TX_OVERFLOW_LSB	0
+#define MBOX_ERROR_INT_STATUS_TX_OVERFLOW_MASK	0x00000001
+#define MBOX_COUNTER_INT_STATUS_ADDRESS		0x00000803
+#define MBOX_COUNTER_INT_STATUS_COUNTER_LSB	0
+#define MBOX_COUNTER_INT_STATUS_COUNTER_MASK	0x000000ff
+#define MBOX_RX_LOOKAHEAD_VALID_ADDRESS		0x00000805
+#define MBOX_INT_STATUS_ENABLE_ADDRESS		0x00000828
+#define MBOX_INT_STATUS_ENABLE_ERROR_LSB	7
+#define MBOX_INT_STATUS_ENABLE_ERROR_MASK	0x00000080
+#define MBOX_INT_STATUS_ENABLE_CPU_LSB		6
+#define MBOX_INT_STATUS_ENABLE_CPU_MASK		0x00000040
+#define MBOX_INT_STATUS_ENABLE_INT_LSB		5
+#define MBOX_INT_STATUS_ENABLE_INT_MASK		0x00000020
+#define MBOX_INT_STATUS_ENABLE_COUNTER_LSB	4
+#define MBOX_INT_STATUS_ENABLE_COUNTER_MASK	0x00000010
+#define MBOX_INT_STATUS_ENABLE_MBOX_DATA_LSB	0
+#define MBOX_INT_STATUS_ENABLE_MBOX_DATA_MASK	0x0000000f
+#define MBOX_CPU_INT_STATUS_ENABLE_ADDRESS	0x00000819
+#define MBOX_CPU_INT_STATUS_ENABLE_BIT_LSB	0
+#define MBOX_CPU_INT_STATUS_ENABLE_BIT_MASK	0x000000ff
+#define MBOX_ERROR_STATUS_ENABLE_ADDRESS	0x0000081a
+#define MBOX_ERROR_STATUS_ENABLE_RX_UNDERFLOW_LSB  1
+#define MBOX_ERROR_STATUS_ENABLE_RX_UNDERFLOW_MASK 0x00000002
+#define MBOX_ERROR_STATUS_ENABLE_TX_OVERFLOW_LSB   0
+#define MBOX_ERROR_STATUS_ENABLE_TX_OVERFLOW_MASK  0x00000001
+#define MBOX_COUNTER_INT_STATUS_ENABLE_ADDRESS	0x0000081b
+#define MBOX_COUNTER_INT_STATUS_ENABLE_BIT_LSB	0
+#define MBOX_COUNTER_INT_STATUS_ENABLE_BIT_MASK	0x000000ff
+#define MBOX_COUNT_ADDRESS			0x00000820
+#define MBOX_COUNT_DEC_ADDRESS			0x00000840
+#define MBOX_WINDOW_DATA_ADDRESS		0x00000874
+#define MBOX_WINDOW_WRITE_ADDR_ADDRESS		0x00000878
+#define MBOX_WINDOW_READ_ADDR_ADDRESS		0x0000087c
+#define MBOX_CPU_DBG_SEL_ADDRESS		0x00000883
+#define MBOX_CPU_DBG_ADDRESS			0x00000884
+#define MBOX_RTC_BASE_ADDRESS			0x00000000
+#define MBOX_GPIO_BASE_ADDRESS			0x00005000
+#define MBOX_MBOX_BASE_ADDRESS			0x00008000
+
 #define RTC_STATE_V_GET(x) (((x) & RTC_STATE_V_MASK) >> RTC_STATE_V_LSB)
 
 /* Register definitions for first generation ath10k cards. These cards include
-- 
1.7.9.5

[RFC v2 10/11] ath10k: Added more host_interest members

[Erik Stromdahl]

Augmented struct host_interest with more members.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/targaddrs.h |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/targaddrs.h b/drivers/net/wireless/ath/ath10k/targaddrs.h
index a47cab4..58e0541 100644
--- a/drivers/net/wireless/ath/ath10k/targaddrs.h
+++ b/drivers/net/wireless/ath/ath10k/targaddrs.h
@@ -205,6 +205,24 @@ struct host_interest {
 	 */
 	/* Bit 1 - unused */
 	u32 hi_fw_swap;					/* 0x104 */
+
+	/* global arenas pointer address, used by host driver debug */
+	u32 hi_dynamic_mem_arenas_addr;			/* 0x108 */
+
+	/* allocated bytes of DRAM use by allocated */
+	u32 hi_dynamic_mem_allocated;			/* 0x10C */
+
+	/* remaining bytes of DRAM */
+	u32 hi_dynamic_mem_remaining;			/* 0x110 */
+
+	/* memory track count, configured by host */
+	u32 hi_dynamic_mem_track_max;			/* 0x114 */
+
+	/* minidump buffer */
+	u32 hi_minidump;				/* 0x118 */
+
+	/* bdata's sig and key addr */
+	u32 hi_bd_sig_key;				/* 0x11c */
 } __packed;
 
 #define HI_ITEM(item)  offsetof(struct host_interest, item)
@@ -319,6 +337,12 @@ struct host_interest {
 #define HI_ACS_FLAGS_USE_WWAN       (1 << 1)
 /* Use test VAP */
 #define HI_ACS_FLAGS_TEST_VAP       (1 << 2)
+/* SDIO/mailbox ACS flag definitions */
+#define HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_SET       (1 << 0)
+#define HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET    (1 << 1)
+#define HI_ACS_FLAGS_ALT_DATA_CREDIT_SIZE        (1 << 2)
+#define HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_FW_ACK    (1 << 16)
+#define HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_FW_ACK (1 << 17)
 
 /*
  * CONSOLE FLAGS
-- 
1.7.9.5

[RFC v2 11/11] ath10k: Added sdio support

[Erik Stromdahl]

Initial HIF sdio/mailbox implementation.

Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
 drivers/net/wireless/ath/ath10k/Kconfig  |    6 +
 drivers/net/wireless/ath/ath10k/Makefile |    3 +
 drivers/net/wireless/ath/ath10k/sdio.c   | 1860 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/sdio.h   |  276 +++++
 4 files changed, 2145 insertions(+)
 create mode 100644 drivers/net/wireless/ath/ath10k/sdio.c
 create mode 100644 drivers/net/wireless/ath/ath10k/sdio.h

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig
index db1ca62..9a03178 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -21,6 +21,12 @@ config ATH10K_AHB
 	---help---
 	  This module adds support for AHB bus
 
+config ATH10K_SDIO
+	tristate "Atheros ath10k SDIO support (EXPERIMENTAL)"
+	depends on ATH10K && MMC
+	---help---
+	  This module adds support for SDIO/MMC bus
+
 config ATH10K_DEBUG
 	bool "Atheros ath10k debugging"
 	depends on ATH10K
diff --git a/drivers/net/wireless/ath/ath10k/Makefile b/drivers/net/wireless/ath/ath10k/Makefile
index 930fadd..b0b19a7 100644
--- a/drivers/net/wireless/ath/ath10k/Makefile
+++ b/drivers/net/wireless/ath/ath10k/Makefile
@@ -27,5 +27,8 @@ ath10k_pci-y += pci.o \
 
 ath10k_pci-$(CONFIG_ATH10K_AHB) += ahb.o
 
+obj-$(CONFIG_ATH10K_SDIO) += ath10k_sdio.o
+ath10k_sdio-y += sdio.o
+
 # for tracing framework to find trace.h
 CFLAGS_trace.o := -I$(src)
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
new file mode 100644
index 0000000..4c123d9
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -0,0 +1,1860 @@
+/*
+ * Copyright (c) 2004-2011 Atheros Communications Inc.
+ * Copyright (c) 2011-2012 Qualcomm Atheros, Inc.
+ * Copyright (c) 2016 Kapsch Trafficcom AB
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <linux/module.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/sdio_func.h>
+#include <linux/mmc/sdio_ids.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/sd.h>
+#include "core.h"
+#include "bmi.h"
+#include "debug.h"
+#include "hif.h"
+#include "htc.h"
+#include "targaddrs.h"
+#include "trace.h"
+#include "sdio.h"
+
+#define CALC_TXRX_PADDED_LEN(ar_sdio, len) \
+	(__ALIGN_MASK((len), (ar_sdio)->mbox_info.block_mask))
+
+static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *buf,
+				       u32 len, u32 request);
+static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
+				     size_t buf_len);
+static int ath10k_sdio_hif_diag_read32(struct ath10k *ar, u32 address,
+				       u32 *value);
+
+/* HIF mbox interrupt handling */
+
+static int ath10k_sdio_mbox_rx_process_packet(struct ath10k_sdio *ar_sdio,
+					      struct ath10k_sdio_rx_data *pkt,
+					      u32 *lookaheads,
+					      int *n_lookaheads)
+{
+	int status = 0;
+	struct ath10k_htc *htc = &ar_sdio->ar->htc;
+	struct sk_buff *skb = pkt->skb;
+	struct ath10k_htc_hdr *htc_hdr = (struct ath10k_htc_hdr *)skb->data;
+	bool trailer_present = htc_hdr->flags & ATH10K_HTC_FLAG_TRAILER_PRESENT;
+	u16 payload_len;
+
+	payload_len = le16_to_cpu(htc_hdr->len);
+
+	if (trailer_present) {
+		u8 *trailer;
+		enum ath10k_htc_ep_id eid;
+
+		trailer = skb->data + sizeof(struct ath10k_htc_hdr) +
+			  payload_len - htc_hdr->trailer_len;
+
+		eid = (enum ath10k_htc_ep_id)htc_hdr->eid;
+
+		status = ath10k_htc_process_trailer(htc,
+						    trailer,
+						    htc_hdr->trailer_len,
+						    eid,
+						    lookaheads,
+						    n_lookaheads);
+		if (status)
+			goto err;
+
+		skb_pull(skb, sizeof(*htc_hdr));
+		skb_trim(skb, skb->len - htc_hdr->trailer_len);
+	}
+
+err:
+	return status;
+}
+
+static inline void ath10k_sdio_mbox_free_rx_pkt(struct ath10k_sdio_rx_data *pkt)
+{
+	dev_kfree_skb(pkt->skb);
+	pkt->skb = NULL;
+	pkt->alloc_len = 0;
+	pkt->act_len = 0;
+}
+
+static inline int ath10k_sdio_mbox_alloc_rx_pkt(struct ath10k_sdio_rx_data *pkt,
+						size_t act_len, size_t full_len,
+						bool part_of_bundle,
+						bool last_in_bundle)
+{
+	pkt->skb = dev_alloc_skb(full_len);
+	if (!pkt->skb)
+		return -ENOMEM;
+
+	pkt->act_len = act_len;
+	pkt->alloc_len = full_len;
+	pkt->part_of_bundle = part_of_bundle;
+	pkt->last_in_bundle = last_in_bundle;
+
+	return 0;
+}
+
+static int ath10k_sdio_mbox_rx_process_packets(struct ath10k_sdio *ar_sdio,
+					       u32 lookaheads[],
+					       int *n_lookahead)
+{
+	struct ath10k *ar = ar_sdio->ar;
+	struct ath10k_htc *htc = &ar->htc;
+	struct ath10k_sdio_rx_data *pkt;
+	int status = 0, i;
+
+	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
+		struct ath10k_htc_ep *ep;
+		enum ath10k_htc_ep_id id;
+		u32 *lookaheads_local = lookaheads;
+		int *n_lookahead_local = n_lookahead;
+
+		id = ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
+
+		if (id >= ATH10K_HTC_EP_COUNT) {
+			ath10k_err(ar, "Invalid endpoint in look-ahead: %d\n",
+				   id);
+			status = -ENOMEM;
+			goto out;
+		}
+
+		ep = &htc->endpoint[id];
+
+		if (ep->service_id == 0) {
+			ath10k_err(ar, "ep %d is not connected !\n", id);
+			status = -ENOMEM;
+			goto out;
+		}
+
+		pkt = &ar_sdio->rx_pkts[i];
+
+		if (pkt->part_of_bundle && !pkt->last_in_bundle) {
+			/* Only read lookahead's from RX trailers
+			 * for the last packet in a bundle.
+			 */
+			lookaheads_local = NULL;
+			n_lookahead_local = NULL;
+		}
+
+		status = ath10k_sdio_mbox_rx_process_packet(ar_sdio,
+							    pkt,
+							    lookaheads_local,
+							    n_lookahead_local);
+		if (status)
+			goto out;
+
+		ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
+		/* The RX complete handler now owns the skb...*/
+		pkt->skb = NULL;
+		pkt->alloc_len = 0;
+	}
+
+out:
+	/* Free all packets that was not passed on to the RX completion
+	 * handler...
+	 */
+	for (; i < ar_sdio->n_rx_pkts; i++)
+		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
+
+	return status;
+}
+
+static int 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)
+{
+	int i, status = 0;
+
+	*bndl_cnt = (htc_hdr->flags & ATH10K_HTC_FLAG_BUNDLE_MASK) >>
+		    ATH10K_HTC_FLAG_BUNDLE_LSB;
+
+	if (*bndl_cnt > HTC_HOST_MAX_MSG_PER_BUNDLE) {
+		ath10k_err(ar,
+			   "HTC bundle len %u exceeds maximum %u !\n",
+			   le16_to_cpu(htc_hdr->len),
+			   HTC_HOST_MAX_MSG_PER_BUNDLE);
+		status = -ENOMEM;
+		goto out;
+	}
+
+	/* Allocate bndl_cnt extra skb's for the bundle.
+	 * The package containing the
+	 * ATH10K_HTC_FLAG_BUNDLE_MASK flag is not included
+	 * in bndl_cnt. The skb for that packet will be
+	 * allocated separately.
+	 */
+	for (i = 0; i < *bndl_cnt; i++) {
+		status = ath10k_sdio_mbox_alloc_rx_pkt(&rx_pkts[i],
+						       act_len,
+						       full_len,
+						       true,
+						       false);
+		if (status)
+			goto out;
+	}
+
+out:
+	return status;
+}
+
+static int ath10k_sdio_mbox_rx_alloc(struct ath10k_sdio *ar_sdio,
+				     u32 lookaheads[], int n_lookaheads)
+{
+	int status = 0, i;
+	struct ath10k *ar = ar_sdio->ar;
+
+	if (n_lookaheads > ATH10K_SDIO_MAX_RX_MSGS) {
+		ath10k_err(ar,
+			   "The total number of pkgs to be fetched (%u) exceeds maximum %u !\n",
+			   n_lookaheads,
+			   ATH10K_SDIO_MAX_RX_MSGS);
+		status = -ENOMEM;
+		goto err;
+	}
+
+	for (i = 0; i < n_lookaheads; i++) {
+		struct ath10k_htc_hdr *htc_hdr =
+			(struct ath10k_htc_hdr *)&lookaheads[i];
+		size_t full_len, act_len;
+		bool last_in_bundle = false;
+
+		if (le16_to_cpu(htc_hdr->len) >
+		    ATH10K_HTC_MBOX_MAX_PAYLOAD_LENGTH) {
+			ath10k_err(ar,
+				   "payload len %d exceeds max htc : %u!\n",
+				   le16_to_cpu(htc_hdr->len),
+				   ATH10K_HTC_MBOX_MAX_PAYLOAD_LENGTH);
+			status = -ENOMEM;
+			goto err;
+		}
+
+		act_len = le16_to_cpu(htc_hdr->len) + sizeof(*htc_hdr);
+		full_len = CALC_TXRX_PADDED_LEN(ar_sdio, act_len);
+
+		if (full_len > ATH10K_SDIO_MAX_BUFFER_SIZE) {
+			ath10k_warn(ar,
+				    "Rx buffer requested with invalid length htc_hdr:eid %d, flags 0x%x, len %d\n",
+				    htc_hdr->eid, htc_hdr->flags,
+				    le16_to_cpu(htc_hdr->len));
+			status = -EINVAL;
+			goto err;
+		}
+
+		if (htc_hdr->flags & ATH10K_HTC_FLAG_BUNDLE_MASK) {
+			/* HTC header indicates that every packet to follow
+			 * has the same padded length so that it can be
+			 * optimally fetched as a full bundle.
+			 */
+			size_t bndl_cnt;
+
+			status = alloc_pkt_bundle(ar, &ar_sdio->rx_pkts[i],
+						  htc_hdr,
+						  full_len, act_len, &bndl_cnt);
+
+			n_lookaheads += bndl_cnt;
+			i += bndl_cnt;
+			/*Next buffer will be the last in the bundle */
+			last_in_bundle = true;
+		}
+
+		/* Allocate skb for packet. If the packet had the
+		 * ATH10K_HTC_FLAG_BUNDLE_MASK flag set, all bundled
+		 * packet skb's have been allocated in the previous step.
+		 */
+		status = ath10k_sdio_mbox_alloc_rx_pkt(&ar_sdio->rx_pkts[i],
+						       act_len,
+						       full_len,
+						       last_in_bundle,
+						       last_in_bundle);
+	}
+
+	ar_sdio->n_rx_pkts = i;
+
+	return 0;
+err:
+
+	for (i = 0; i < ATH10K_SDIO_MAX_RX_MSGS; i++) {
+		if (!ar_sdio->rx_pkts[i].alloc_len)
+			break;
+		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
+	}
+
+	return status;
+}
+
+static int ath10k_sdio_mbox_rx_packet(struct ath10k_sdio *ar_sdio,
+				      struct ath10k_sdio_rx_data *pkt)
+{
+	struct ath10k *ar = ar_sdio->ar;
+	struct sk_buff *skb = pkt->skb;
+	int status;
+
+	status = ath10k_sdio_read_write_sync(ar,
+					     ar_sdio->mbox_info.htc_addr,
+					     skb->data, pkt->alloc_len,
+					     HIF_RD_SYNC_BLOCK_FIX);
+
+	pkt->status = status;
+	if (!status)
+		skb_put(skb, pkt->act_len);
+
+	return status;
+}
+
+static int ath10k_sdio_mbox_rx_fetch(struct ath10k_sdio *ar_sdio)
+{
+	int i, status = 0;
+
+	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
+		status = ath10k_sdio_mbox_rx_packet(ar_sdio,
+						    &ar_sdio->rx_pkts[i]);
+		if (status)
+			goto err;
+	}
+
+	return 0;
+err:
+	/* Free all packets that was not successfully fetched. */
+	for (; i < ar_sdio->n_rx_pkts; i++)
+		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
+
+	return status;
+}
+
+/* Disable packet reception (used in case the host runs out of buffers)
+ * using the interrupt enable registers through the host I/F
+ */
+static int ath10k_sdio_hif_rx_control(struct ath10k_sdio *ar_sdio,
+				      bool enable_rx)
+{
+	int status = 0;
+	struct ath10k_sdio_irq_enable_reg regs;
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+
+	ath10k_dbg(ar_sdio->ar, ATH10K_DBG_SDIO, "hif rx %s\n",
+		   enable_rx ? "enable" : "disable");
+
+	spin_lock_bh(&irq_data->lock);
+
+	if (enable_rx)
+		irq_data->irq_en_reg.int_status_en |=
+			SM(0x01, MBOX_INT_STATUS_ENABLE_MBOX_DATA);
+	else
+		irq_data->irq_en_reg.int_status_en &=
+			~SM(0x01, MBOX_INT_STATUS_ENABLE_MBOX_DATA);
+
+	regs = irq_data->irq_en_reg;
+
+	spin_unlock_bh(&irq_data->lock);
+
+	status = ath10k_sdio_read_write_sync(ar_sdio->ar,
+					     MBOX_INT_STATUS_ENABLE_ADDRESS,
+					     &regs.int_status_en, sizeof(regs),
+					     HIF_WR_SYNC_BYTE_INC);
+
+	return status;
+}
+
+int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k_sdio *ar_sdio,
+					   u32 msg_lookahead, bool *done)
+{
+	struct ath10k *ar = ar_sdio->ar;
+	int status = 0;
+	u32 lookaheads[ATH10K_SDIO_MAX_RX_MSGS];
+	int n_lookaheads = 1;
+
+	*done = true;
+
+	/* Copy the lookahead obtained from the HTC register table into our
+	 * temp array as a start value.
+	 */
+	lookaheads[0] = msg_lookahead;
+
+	for (;;) {
+		/* Try to allocate as many HTC RX packets indicated by
+		 * n_lookaheads.
+		 */
+		status = ath10k_sdio_mbox_rx_alloc(ar_sdio, lookaheads,
+						   n_lookaheads);
+		if (status)
+			break;
+
+		if (ar_sdio->n_rx_pkts >= 2)
+			/* A recv bundle was detected, force IRQ status
+			 * re-check again.
+			 */
+			*done = false;
+
+		status = ath10k_sdio_mbox_rx_fetch(ar_sdio);
+
+		/* Process fetched packets. This will potentially update
+		 * n_lookaheads depending on if the packets contain lookahead
+		 * reports.
+		 */
+		n_lookaheads = 0;
+		status = ath10k_sdio_mbox_rx_process_packets(ar_sdio,
+							     lookaheads,
+							     &n_lookaheads);
+
+		if (!n_lookaheads || status)
+			break;
+
+		/* For SYNCH processing, if we get here, we are running
+		 * through the loop again due to updated lookaheads. Set
+		 * flag that we should re-check IRQ status registers again
+		 * before leaving IRQ processing, this can net better
+		 * performance in high throughput situations.
+		 */
+		*done = false;
+	}
+
+	if (status && (status != -ECANCELED))
+		ath10k_err(ar, "failed to get pending recv messages: %d\n",
+			   status);
+
+	if (atomic_read(&ar_sdio->stopping)) {
+		ath10k_warn(ar, "host is going to stop. Turning of RX\n");
+		ath10k_sdio_hif_rx_control(ar_sdio, false);
+	}
+
+	return status;
+}
+
+static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k_sdio *ar_sdio)
+{
+	int ret;
+	u32 dummy;
+	struct ath10k *ar = ar_sdio->ar;
+
+	ath10k_warn(ar, "firmware crashed\n");
+
+	/* read counter to clear the interrupt, the debug error interrupt is
+	 * counter 0.
+	 */
+	ret = ath10k_sdio_read_write_sync(ar, MBOX_COUNT_DEC_ADDRESS,
+					  (u8 *)&dummy, 4,
+					  HIF_RD_SYNC_BYTE_INC);
+	if (ret)
+		ath10k_warn(ar, "Failed to clear debug interrupt: %d\n", ret);
+
+	return ret;
+}
+
+static int ath10k_sdio_mbox_proc_counter_intr(struct ath10k_sdio *ar_sdio)
+{
+	u8 counter_int_status;
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+
+	counter_int_status = irq_data->irq_proc_reg.counter_int_status &
+			     irq_data->irq_en_reg.cntr_int_status_en;
+
+	/* NOTE: other modules like GMBOX may use the counter interrupt for
+	 * credit flow control on other counters, we only need to check for
+	 * the debug assertion counter interrupt.
+	 */
+	if (counter_int_status & ATH10K_SDIO_TARGET_DEBUG_INTR_MASK)
+		return ath10k_sdio_mbox_proc_dbg_intr(ar_sdio);
+
+	return 0;
+}
+
+static int ath10k_sdio_mbox_proc_err_intr(struct ath10k_sdio *ar_sdio)
+{
+	int status;
+	u8 error_int_status;
+	u8 reg_buf[4];
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+	struct ath10k *ar = ar_sdio->ar;
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "error interrupt\n");
+
+	error_int_status = irq_data->irq_proc_reg.error_int_status & 0x0F;
+	if (!error_int_status) {
+		WARN_ON(1);
+		return -EIO;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO,
+		   "valid interrupt source(s) in ERROR_INT_STATUS: 0x%x\n",
+		   error_int_status);
+
+	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_WAKEUP))
+		ath10k_dbg(ar, ATH10K_DBG_SDIO, "error : wakeup\n");
+
+	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_RX_UNDERFLOW))
+		ath10k_err(ar, "rx underflow\n");
+
+	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_TX_OVERFLOW))
+		ath10k_err(ar, "tx overflow\n");
+
+	/* Clear the interrupt */
+	irq_data->irq_proc_reg.error_int_status &= ~error_int_status;
+
+	/* set W1C value to clear the interrupt, this hits the register first */
+	reg_buf[0] = error_int_status;
+	reg_buf[1] = 0;
+	reg_buf[2] = 0;
+	reg_buf[3] = 0;
+
+	status = ath10k_sdio_read_write_sync(ar,
+					     MBOX_ERROR_INT_STATUS_ADDRESS,
+					     reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
+
+	WARN_ON(status);
+
+	return status;
+}
+
+static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k_sdio *ar_sdio)
+{
+	int status;
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+	struct ath10k *ar = ar_sdio->ar;
+	u8 cpu_int_status, reg_buf[4];
+
+	cpu_int_status = irq_data->irq_proc_reg.cpu_int_status &
+			 irq_data->irq_en_reg.cpu_int_status_en;
+	if (!cpu_int_status) {
+		WARN_ON(1);
+		return -EIO;
+	}
+
+	/* Clear the interrupt */
+	irq_data->irq_proc_reg.cpu_int_status &= ~cpu_int_status;
+
+	/* Set up the register transfer buffer to hit the register 4 times ,
+	 * this is done to make the access 4-byte aligned to mitigate issues
+	 * with host bus interconnects that restrict bus transfer lengths to
+	 * be a multiple of 4-bytes.
+	 */
+
+	/* set W1C value to clear the interrupt, this hits the register first */
+	reg_buf[0] = cpu_int_status;
+	/* the remaining are set to zero which have no-effect  */
+	reg_buf[1] = 0;
+	reg_buf[2] = 0;
+	reg_buf[3] = 0;
+
+	status = ath10k_sdio_read_write_sync(ar,
+					     MBOX_CPU_INT_STATUS_ADDRESS,
+					     reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
+
+	WARN_ON(status);
+
+	return status;
+}
+
+/* process pending interrupts synchronously */
+static int ath10k_sdio_mbox_proc_pending_irqs(struct ath10k_sdio *ar_sdio,
+					      bool *done)
+{
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+	struct ath10k *ar = ar_sdio->ar;
+	struct ath10k_sdio_irq_proc_registers *rg;
+	int status = 0;
+	u8 host_int_status = 0;
+	u32 lookahead = 0;
+	u8 htc_mbox = 1 << ATH10K_HTC_MAILBOX;
+
+	/* NOTE: HIF implementation guarantees that the context of this
+	 * call allows us to perform SYNCHRONOUS I/O, that is we can block,
+	 * sleep or call any API that can block or switch thread/task
+	 * contexts. This is a fully schedulable context.
+	 */
+
+	/* Process pending intr only when int_status_en is clear, it may
+	 * result in unnecessary bus transaction otherwise. Target may be
+	 * unresponsive at the time.
+	 */
+	if (irq_data->irq_en_reg.int_status_en) {
+		/* Read the first sizeof(struct ath10k_irq_proc_registers)
+		 * bytes of the HTC register table. This
+		 * will yield us the value of different int status
+		 * registers and the lookahead registers.
+		 */
+		status = ath10k_sdio_read_write_sync(
+				ar,
+				MBOX_HOST_INT_STATUS_ADDRESS,
+				(u8 *)&irq_data->irq_proc_reg,
+				sizeof(irq_data->irq_proc_reg),
+				HIF_RD_SYNC_BYTE_INC);
+		if (status)
+			goto out;
+
+		/* Update only those registers that are enabled */
+		host_int_status = irq_data->irq_proc_reg.host_int_status &
+				  irq_data->irq_en_reg.int_status_en;
+
+		/* Look at mbox status */
+		if (host_int_status & htc_mbox) {
+			/* Mask out pending mbox value, we use look ahead as
+			 * the real flag for mbox processing.
+			 */
+			host_int_status &= ~htc_mbox;
+			if (irq_data->irq_proc_reg.rx_lookahead_valid &
+			    htc_mbox) {
+				rg = &irq_data->irq_proc_reg;
+				lookahead = le32_to_cpu(
+					rg->rx_lookahead[ATH10K_HTC_MAILBOX]);
+				if (!lookahead)
+					ath10k_err(ar, "lookahead is zero!\n");
+			}
+		}
+	}
+
+	if (!host_int_status && !lookahead) {
+		*done = true;
+		goto out;
+	}
+
+	if (lookahead) {
+		ath10k_dbg(ar, ATH10K_DBG_SDIO,
+			   "pending mailbox msg, lookahead: 0x%08X\n",
+			   lookahead);
+
+		status = ath10k_sdio_mbox_rxmsg_pending_handler(ar_sdio,
+								lookahead,
+								done);
+		if (status)
+			goto out;
+	}
+
+	/* now, handle the rest of the interrupts */
+	ath10k_dbg(ar, ATH10K_DBG_SDIO,
+		   "valid interrupt source(s) for other interrupts: 0x%x\n",
+		   host_int_status);
+
+	if (MS(host_int_status, MBOX_HOST_INT_STATUS_CPU)) {
+		/* CPU Interrupt */
+		status = ath10k_sdio_mbox_proc_cpu_intr(ar_sdio);
+		if (status)
+			goto out;
+	}
+
+	if (MS(host_int_status, MBOX_HOST_INT_STATUS_ERROR)) {
+		/* Error Interrupt */
+		status = ath10k_sdio_mbox_proc_err_intr(ar_sdio);
+		if (status)
+			goto out;
+	}
+
+	if (MS(host_int_status, MBOX_HOST_INT_STATUS_COUNTER))
+		/* Counter Interrupt */
+		status = ath10k_sdio_mbox_proc_counter_intr(ar_sdio);
+
+out:
+	/* An optimization to bypass reading the IRQ status registers
+	 * unecessarily which can re-wake the target, if upper layers
+	 * determine that we are in a low-throughput mode, we can rely on
+	 * taking another interrupt rather than re-checking the status
+	 * registers which can re-wake the target.
+	 *
+	 * NOTE : for host interfaces that makes use of detecting pending
+	 * mbox messages at hif can not use this optimization due to
+	 * possible side effects, SPI requires the host to drain all
+	 * messages from the mailbox before exiting the ISR routine.
+	 */
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO,
+		   "%s: (done:%d, status=%d)\n", __func__, *done, status);
+
+	return status;
+}
+
+/* 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(u8 *buf)
+{
+	return ((unsigned long)buf & 0x3) || !virt_addr_valid(buf);
+}
+
+static inline enum ath10k_htc_ep_id pipe_id_to_eid(u8 pipe_id)
+{
+	return (enum ath10k_htc_ep_id)pipe_id;
+}
+
+static void ath10k_sdio_set_mbox_info(struct ath10k_sdio *ar_sdio)
+{
+	struct ath10k_mbox_info *mbox_info = &ar_sdio->mbox_info;
+	u16 device = ar_sdio->func->device;
+
+	mbox_info->htc_addr = ATH10K_HIF_MBOX_BASE_ADDR;
+	mbox_info->block_size = ATH10K_HIF_MBOX_BLOCK_SIZE;
+	mbox_info->block_mask = ATH10K_HIF_MBOX_BLOCK_SIZE - 1;
+	mbox_info->gmbox_addr = ATH10K_HIF_GMBOX_BASE_ADDR;
+	mbox_info->gmbox_sz = ATH10K_HIF_GMBOX_WIDTH;
+
+	mbox_info->ext_info[0].htc_ext_addr = ATH10K_HIF_MBOX0_EXT_BASE_ADDR;
+
+	if ((device & ATH10K_MANUFACTURER_ID_REV_MASK) < 4)
+		mbox_info->ext_info[0].htc_ext_sz = ATH10K_HIF_MBOX0_EXT_WIDTH;
+	else
+		/* from rome 2.0(0x504), the width has been extended
+		 * to 56K
+		 */
+		mbox_info->ext_info[0].htc_ext_sz =
+			ATH10K_HIF_MBOX0_EXT_WIDTH_ROME_2_0;
+
+	mbox_info->ext_info[1].htc_ext_addr =
+		mbox_info->ext_info[0].htc_ext_addr +
+		mbox_info->ext_info[0].htc_ext_sz +
+		ATH10K_HIF_MBOX_DUMMY_SPACE_SIZE;
+	mbox_info->ext_info[1].htc_ext_sz = ATH10K_HIF_MBOX1_EXT_WIDTH;
+}
+
+static inline void ath10k_sdio_set_cmd52_arg(u32 *arg, u8 write, u8 raw,
+					     unsigned int address,
+					     unsigned char val)
+{
+	const u8 func = 0;
+
+	*arg = ((write & 1) << 31) |
+	       ((func & 0x7) << 28) |
+	       ((raw & 1) << 27) |
+	       (1 << 26) |
+	       ((address & 0x1FFFF) << 9) |
+	       (1 << 8) |
+	       (val & 0xFF);
+}
+
+static int ath10k_sdio_func0_cmd52_wr_byte(struct mmc_card *card,
+					   unsigned int address,
+					   unsigned char byte)
+{
+	struct mmc_command io_cmd;
+
+	memset(&io_cmd, 0, sizeof(io_cmd));
+	ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 1, 0, address, byte);
+	io_cmd.opcode = SD_IO_RW_DIRECT;
+	io_cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
+
+	return mmc_wait_for_cmd(card->host, &io_cmd, 0);
+}
+
+static int ath10k_sdio_func0_cmd52_rd_byte(struct mmc_card *card,
+					   unsigned int address,
+					   unsigned char *byte)
+{
+	int ret;
+	struct mmc_command io_cmd;
+
+	memset(&io_cmd, 0, sizeof(io_cmd));
+	ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 0, 0, address, 0);
+	io_cmd.opcode = SD_IO_RW_DIRECT;
+	io_cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
+
+	ret = mmc_wait_for_cmd(card->host, &io_cmd, 0);
+	if (!ret)
+		*byte = io_cmd.resp[0];
+
+	return ret;
+}
+
+static int ath10k_sdio_io(struct ath10k_sdio *ar_sdio, u32 request, u32 addr,
+			  u8 *buf, u32 len)
+{
+	int ret = 0;
+	struct sdio_func *func = ar_sdio->func;
+	struct ath10k *ar = ar_sdio->ar;
+
+	sdio_claim_host(func);
+
+	if (request & HIF_WRITE) {
+		if (request & HIF_FIXED_ADDRESS)
+			ret = sdio_writesb(func, addr, buf, len);
+		else
+			ret = sdio_memcpy_toio(func, addr, buf, len);
+	} else {
+		if (request & HIF_FIXED_ADDRESS)
+			ret = sdio_readsb(func, buf, addr, len);
+		else
+			ret = sdio_memcpy_fromio(func, buf, addr, len);
+	}
+
+	sdio_release_host(func);
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "%s addr 0x%x%s buf 0x%p len %d\n",
+		   request & HIF_WRITE ? "wr" : "rd", addr,
+		   request & HIF_FIXED_ADDRESS ? " (fixed)" : "", buf, len);
+	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL,
+			request & HIF_WRITE ? "sdio wr " : "sdio rd ",
+			buf, len);
+
+	return ret;
+}
+
+static struct ath10k_sdio_bus_request
+*ath10k_sdio_alloc_busreq(struct ath10k_sdio *ar_sdio)
+{
+	struct ath10k_sdio_bus_request *bus_req;
+
+	spin_lock_bh(&ar_sdio->lock);
+
+	if (list_empty(&ar_sdio->bus_req_freeq)) {
+		spin_unlock_bh(&ar_sdio->lock);
+		return NULL;
+	}
+
+	bus_req = list_first_entry(&ar_sdio->bus_req_freeq,
+				   struct ath10k_sdio_bus_request, list);
+	list_del(&bus_req->list);
+
+	spin_unlock_bh(&ar_sdio->lock);
+
+	return bus_req;
+}
+
+static void ath10k_sdio_free_bus_req(struct ath10k_sdio *ar_sdio,
+				     struct ath10k_sdio_bus_request *bus_req)
+{
+	spin_lock_bh(&ar_sdio->lock);
+	list_add_tail(&bus_req->list, &ar_sdio->bus_req_freeq);
+	spin_unlock_bh(&ar_sdio->lock);
+}
+
+static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *buf,
+				       u32 len, u32 request)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	u8  *tbuf = NULL;
+	int ret;
+	bool bounced = false;
+
+	if (request & HIF_BLOCK_BASIS)
+		len = round_down(len, ar_sdio->mbox_info.block_size);
+
+	if (buf_needs_bounce(buf)) {
+		if (!ar_sdio->dma_buffer)
+			return -ENOMEM;
+		/* FIXME: I am not sure if it is always correct to assume
+		 * that the SDIO irq is a "fake" irq and sleep is possible.
+		 * (this function will get called from
+		 * ath10k_sdio_irq_handler
+		 */
+		mutex_lock(&ar_sdio->dma_buffer_mutex);
+		tbuf = ar_sdio->dma_buffer;
+
+		if (request & HIF_WRITE)
+			memcpy(tbuf, buf, len);
+
+		bounced = true;
+	} else {
+		tbuf = buf;
+	}
+
+	ret = ath10k_sdio_io(ar_sdio, request, addr, tbuf, len);
+	if ((request & HIF_READ) && bounced)
+		memcpy(buf, tbuf, len);
+
+	if (bounced)
+		mutex_unlock(&ar_sdio->dma_buffer_mutex);
+
+	return ret;
+}
+
+static void __ath10k_sdio_write_async(struct ath10k_sdio *ar_sdio,
+				      struct ath10k_sdio_bus_request *req)
+{
+	int status;
+	struct ath10k_htc_ep *ep;
+	struct sk_buff *skb;
+
+	skb = req->skb;
+	status = ath10k_sdio_read_write_sync(ar_sdio->ar, req->address,
+					     skb->data, req->len,
+					     req->request);
+	ep = &ar_sdio->ar->htc.endpoint[req->eid];
+	ath10k_htc_notify_tx_completion(ep, skb);
+	ath10k_sdio_free_bus_req(ar_sdio, req);
+}
+
+static void ath10k_sdio_write_async_work(struct work_struct *work)
+{
+	struct ath10k_sdio *ar_sdio;
+	struct ath10k_sdio_bus_request *req, *tmp_req;
+
+	ar_sdio = container_of(work, struct ath10k_sdio, wr_async_work);
+
+	spin_lock_bh(&ar_sdio->wr_async_lock);
+	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
+		list_del(&req->list);
+		spin_unlock_bh(&ar_sdio->wr_async_lock);
+		__ath10k_sdio_write_async(ar_sdio, req);
+		spin_lock_bh(&ar_sdio->wr_async_lock);
+	}
+	spin_unlock_bh(&ar_sdio->wr_async_lock);
+}
+
+static void ath10k_sdio_irq_handler(struct sdio_func *func)
+{
+	int status = 0;
+	unsigned long timeout;
+	struct ath10k_sdio *ar_sdio;
+	bool done = false;
+
+	ar_sdio = sdio_get_drvdata(func);
+	atomic_set(&ar_sdio->irq_handling, 1);
+
+	/* Release the host during interrupts so we can pick it back up when
+	 * we process commands.
+	 */
+	sdio_release_host(ar_sdio->func);
+
+	timeout = jiffies + ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ;
+	while (time_before(jiffies, timeout) && !done) {
+		status = ath10k_sdio_mbox_proc_pending_irqs(ar_sdio, &done);
+		if (status)
+			break;
+	}
+
+	sdio_claim_host(ar_sdio->func);
+
+	atomic_set(&ar_sdio->irq_handling, 0);
+	wake_up(&ar_sdio->irq_wq);
+
+	WARN_ON(status && status != -ECANCELED);
+}
+
+static int ath10k_sdio_hif_disable_intrs(struct ath10k_sdio *ar_sdio)
+{
+	int ret;
+	struct ath10k_sdio_irq_enable_reg regs;
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+
+	memset(&regs, 0, sizeof(regs));
+
+	ret = ath10k_sdio_read_write_sync(ar_sdio->ar,
+					  MBOX_INT_STATUS_ENABLE_ADDRESS,
+					  &regs.int_status_en, sizeof(regs),
+					  HIF_WR_SYNC_BYTE_INC);
+	if (ret) {
+		ath10k_err(ar_sdio->ar, "Unable to disable sdio interrupts\n");
+		return ret;
+	}
+
+	spin_lock_bh(&irq_data->lock);
+	irq_data->irq_en_reg = regs;
+	spin_unlock_bh(&irq_data->lock);
+
+	return 0;
+}
+
+static int ath10k_sdio_hif_power_up(struct ath10k *ar)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	int ret = 0;
+
+	if (!ar_sdio->is_disabled)
+		return 0;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power on\n");
+
+	sdio_claim_host(func);
+
+	ret = sdio_enable_func(func);
+	if (ret) {
+		ath10k_err(ar, "Unable to enable sdio func: %d)\n", ret);
+		sdio_release_host(func);
+		return ret;
+	}
+
+	sdio_release_host(func);
+
+	/* Wait for hardware to initialise. It should take a lot less than
+	 * 20 ms but let's be conservative here.
+	 */
+	msleep(20);
+
+	ar_sdio->is_disabled = false;
+
+	ret = ath10k_sdio_hif_disable_intrs(ar_sdio);
+
+	return ret;
+}
+
+static void ath10k_sdio_hif_power_down(struct ath10k *ar)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	int ret;
+
+	if (ar_sdio->is_disabled)
+		return;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power off\n");
+
+	/* Disable the card */
+	sdio_claim_host(ar_sdio->func);
+	ret = sdio_disable_func(ar_sdio->func);
+	sdio_release_host(ar_sdio->func);
+
+	if (ret)
+		ath10k_dbg(ar, ATH10K_DBG_BOOT,
+			   "Unable to disable sdio: %d\n", ret);
+
+	ar_sdio->is_disabled = true;
+}
+
+int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
+			  struct ath10k_hif_sg_item *items, int n_items)
+{
+	int i;
+	u32 address;
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct ath10k_sdio_bus_request *bus_req;
+
+	bus_req = ath10k_sdio_alloc_busreq(ar_sdio);
+
+	if (WARN_ON_ONCE(!bus_req))
+		return -ENOMEM;
+
+	for (i = 0; i < n_items; i++) {
+		bus_req->skb = items[i].transfer_context;
+		bus_req->request = HIF_WRITE;
+		bus_req->eid = pipe_id_to_eid(pipe_id);
+		/* Write TX data to the end of the mbox address space */
+		address = ar_sdio->mbox_addr[bus_req->eid] +
+			  ar_sdio->mbox_size[bus_req->eid] - bus_req->skb->len;
+		bus_req->address = address;
+		bus_req->len = CALC_TXRX_PADDED_LEN(ar_sdio, bus_req->skb->len);
+
+		spin_lock_bh(&ar_sdio->wr_async_lock);
+		list_add_tail(&bus_req->list, &ar_sdio->wr_asyncq);
+		spin_unlock_bh(&ar_sdio->wr_async_lock);
+	}
+
+	queue_work(ar_sdio->workqueue, &ar_sdio->wr_async_work);
+
+	return 0;
+}
+
+static int ath10k_sdio_hif_enable_intrs(struct ath10k_sdio *ar_sdio)
+{
+	struct ath10k_sdio_irq_enable_reg regs;
+	int status;
+	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
+
+	memset(&regs, 0, sizeof(regs));
+
+	/* Enable all but CPU interrupts */
+	regs.int_status_en = SM(0x01, MBOX_INT_STATUS_ENABLE_ERROR) |
+			     SM(0x01, MBOX_INT_STATUS_ENABLE_CPU) |
+			     SM(0x01, MBOX_INT_STATUS_ENABLE_COUNTER);
+
+	/* NOTE: There are some cases where HIF can do detection of
+	 * pending mbox messages which is disabled now.
+	 */
+	regs.int_status_en |= SM(0x01, MBOX_INT_STATUS_ENABLE_MBOX_DATA);
+
+	/* Set up the CPU Interrupt status Register */
+	regs.cpu_int_status_en = 0;
+
+	/* Set up the Error Interrupt status Register */
+	regs.err_int_status_en =
+		SM(0x01, MBOX_ERROR_STATUS_ENABLE_RX_UNDERFLOW) |
+		SM(0x01, MBOX_ERROR_STATUS_ENABLE_TX_OVERFLOW);
+
+	/* Enable Counter interrupt status register to get fatal errors for
+	 * debugging.
+	 */
+	regs.cntr_int_status_en = SM(ATH10K_SDIO_TARGET_DEBUG_INTR_MASK,
+				     MBOX_COUNTER_INT_STATUS_ENABLE_BIT);
+
+	status = ath10k_sdio_read_write_sync(ar_sdio->ar,
+					     MBOX_INT_STATUS_ENABLE_ADDRESS,
+					     &regs.int_status_en, sizeof(regs),
+					     HIF_WR_SYNC_BYTE_INC);
+	if (status) {
+		ath10k_err(ar_sdio->ar,
+			   "failed to update interrupt ctl reg err: %d\n",
+			   status);
+		return status;
+	}
+
+	spin_lock_bh(&irq_data->lock);
+	irq_data->irq_en_reg = regs;
+	spin_unlock_bh(&irq_data->lock);
+
+	return 0;
+}
+
+static int ath10k_sdio_hif_start(struct ath10k *ar)
+{
+	int ret;
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	u32 addr, val;
+
+	addr = host_interest_item_address(HI_ITEM(hi_acs_flags));
+
+	ret = ath10k_sdio_hif_diag_read32(ar, addr, &val);
+	if (ret) {
+		ath10k_err(ar, "Unable to read diag mem: %d\n", ret);
+		goto out;
+	}
+
+	if (val & HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_FW_ACK) {
+		ath10k_dbg(ar, ATH10K_DBG_SDIO,
+			   "Mailbox SWAP Service is enabled\n");
+		ar_sdio->swap_mbox = true;
+	}
+
+	/* eid 0 always uses the lower part of the extended mailbox address
+	 * space (ext_info[0].htc_ext_addr).
+	 */
+	ar_sdio->mbox_addr[0] = ar_sdio->mbox_info.ext_info[0].htc_ext_addr;
+	ar_sdio->mbox_size[0] = ar_sdio->mbox_info.ext_info[0].htc_ext_sz;
+
+	sdio_claim_host(ar_sdio->func);
+
+	/* Register the isr */
+	ret =  sdio_claim_irq(ar_sdio->func, ath10k_sdio_irq_handler);
+	if (ret) {
+		ath10k_err(ar, "Failed to claim sdio irq: %d\n", ret);
+		sdio_release_host(ar_sdio->func);
+		goto out;
+	}
+
+	sdio_release_host(ar_sdio->func);
+
+	ret = ath10k_sdio_hif_enable_intrs(ar_sdio);
+	if (ret)
+		ath10k_err(ar, "Failed to enable sdio interrupts: %d\n", ret);
+
+out:
+	return ret;
+}
+
+static bool ath10k_sdio_is_on_irq(struct ath10k *ar)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+
+	return !atomic_read(&ar_sdio->irq_handling);
+}
+
+static void ath10k_sdio_irq_disable(struct ath10k *ar)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	int ret;
+
+	sdio_claim_host(ar_sdio->func);
+
+	atomic_set(&ar_sdio->stopping, 1);
+
+	if (atomic_read(&ar_sdio->irq_handling)) {
+		sdio_release_host(ar_sdio->func);
+
+		ret = wait_event_interruptible(ar_sdio->irq_wq,
+					       ath10k_sdio_is_on_irq(ar));
+		if (ret)
+			return;
+
+		sdio_claim_host(ar_sdio->func);
+	}
+
+	ret = sdio_release_irq(ar_sdio->func);
+	if (ret)
+		ath10k_err(ar, "Failed to release sdio irq: %d\n", ret);
+
+	sdio_release_host(ar_sdio->func);
+}
+
+static int ath10k_sdio_config(struct ath10k_sdio *ar_sdio)
+{
+	struct ath10k *ar = ar_sdio->ar;
+	struct sdio_func *func = ar_sdio->func;
+	unsigned char byte, asyncintdelay = 2;
+	int ret;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "SDIO configuration\n");
+
+	sdio_claim_host(func);
+
+	byte = 0;
+	ret = ath10k_sdio_func0_cmd52_rd_byte(func->card,
+					      SDIO_CCCR_DRIVE_STRENGTH,
+					      &byte);
+
+	byte = (byte & (~(SDIO_DRIVE_DTSx_MASK << SDIO_DRIVE_DTSx_SHIFT))) |
+		SDIO_DTSx_SET_TYPE_D;
+
+	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
+					      SDIO_CCCR_DRIVE_STRENGTH,
+					      byte);
+
+	byte = 0;
+	ret = ath10k_sdio_func0_cmd52_rd_byte(
+		func->card,
+		CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
+		&byte);
+
+	byte |= (CCCR_SDIO_DRIVER_STRENGTH_ENABLE_A |
+		 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);
+	if (ret) {
+		ath10k_err(ar, "Failed to enable driver strengt\n");
+		goto out;
+	}
+
+	byte = 0;
+	ret = ath10k_sdio_func0_cmd52_rd_byte(func->card,
+					      CCCR_SDIO_IRQ_MODE_REG_SDIO3,
+					      &byte);
+
+	byte |= SDIO_IRQ_MODE_ASYNC_4BIT_IRQ_SDIO3;
+
+	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
+					      CCCR_SDIO_IRQ_MODE_REG_SDIO3,
+					      byte);
+	if (ret) {
+		ath10k_err(ar, "Failed to enable 4-bit async irq mode %d\n",
+			   ret);
+		goto out;
+	}
+
+	byte = 0;
+	ret = ath10k_sdio_func0_cmd52_rd_byte(func->card,
+					      CCCR_SDIO_ASYNC_INT_DELAY_ADDRESS,
+					      &byte);
+
+	byte = (byte & ~CCCR_SDIO_ASYNC_INT_DELAY_MASK) |
+		((asyncintdelay << CCCR_SDIO_ASYNC_INT_DELAY_LSB) &
+		CCCR_SDIO_ASYNC_INT_DELAY_MASK);
+
+	ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
+					      CCCR_SDIO_ASYNC_INT_DELAY_ADDRESS,
+					      byte);
+
+	/* give us some time to enable, in ms */
+	func->enable_timeout = 100;
+
+	ret = sdio_set_block_size(func, ar_sdio->mbox_info.block_size);
+	if (ret) {
+		ath10k_err(ar, "Set sdio block size %d failed: %d)\n",
+			   ar_sdio->mbox_info.block_size, ret);
+		goto out;
+	}
+
+out:
+	sdio_release_host(func);
+
+	return ret;
+}
+
+/* set the window address register (using 4-byte register access ). */
+static int ath10k_set_addrwin_reg(struct ath10k *ar, u32 reg_addr, u32 addr)
+{
+	int status;
+
+	status = ath10k_sdio_read_write_sync(ar, reg_addr, (u8 *)(&addr),
+					     4, HIF_WR_SYNC_BYTE_INC);
+
+	if (status) {
+		ath10k_err(ar, "%s: failed to write 0x%x to window reg: 0x%X\n",
+			   __func__, addr, reg_addr);
+		return status;
+	}
+
+	return 0;
+}
+
+static int ath10k_sdio_hif_diag_read32(struct ath10k *ar, u32 address,
+				       u32 *value)
+{
+	__le32 val = 0;
+	int ret;
+
+	ret = ath10k_sdio_hif_diag_read(ar, address, &val, sizeof(val));
+	*value = __le32_to_cpu(val);
+
+	return ret;
+}
+
+static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
+				     size_t buf_len)
+{
+	int status;
+
+	/* set window register to start read cycle */
+	status = ath10k_set_addrwin_reg(ar, MBOX_WINDOW_READ_ADDR_ADDRESS,
+					address);
+
+	if (status)
+		return status;
+
+	/* read the data */
+	status = ath10k_sdio_read_write_sync(ar, MBOX_WINDOW_DATA_ADDRESS,
+					     (u8 *)buf, buf_len,
+					     HIF_RD_SYNC_BYTE_INC);
+	if (status) {
+		ath10k_err(ar, "%s: failed to read from window data addr\n",
+			   __func__);
+		return status;
+	}
+
+	return status;
+}
+
+static int ath10k_sdio_diag_write_mem(struct ath10k *ar, u32 address,
+				      const void *data, int nbytes)
+{
+	int status;
+
+	/* set write data */
+	status = ath10k_sdio_read_write_sync(ar, MBOX_WINDOW_DATA_ADDRESS,
+					     (u8 *)data, nbytes,
+					     HIF_WR_SYNC_BYTE_INC);
+	if (status) {
+		ath10k_err(ar, "%s: failed to write 0x%p to window data addr\n",
+			   __func__, data);
+		return status;
+	}
+
+	/* set window register, which starts the write cycle */
+	return ath10k_set_addrwin_reg(ar, MBOX_WINDOW_WRITE_ADDR_ADDRESS,
+				      address);
+}
+
+static int ath10k_sdio_bmi_credits(struct ath10k *ar)
+{
+	u32 addr, cmd_credits;
+	unsigned long timeout;
+	int ret;
+
+	cmd_credits = 0;
+
+	/* Read the counter register to get the command credits */
+	addr = MBOX_COUNT_DEC_ADDRESS + ATH10K_HIF_MBOX_NUM_MAX * 4;
+
+	timeout = jiffies + BMI_COMMUNICATION_TIMEOUT_HZ;
+	while (time_before(jiffies, timeout) && !cmd_credits) {
+		/* Hit the credit counter with a 4-byte access, the first byte
+		 * read will hit the counter and cause a decrement, while the
+		 * remaining 3 bytes has no effect. The rationale behind this
+		 * is to make all HIF accesses 4-byte aligned.
+		 */
+		ret = ath10k_sdio_read_write_sync(ar, addr,
+						  (u8 *)&cmd_credits,
+						  sizeof(cmd_credits),
+						  HIF_RD_SYNC_BYTE_INC);
+		if (ret) {
+			ath10k_err(ar,
+				   "Unable to decrement the command credit count register: %d\n",
+				   ret);
+			return ret;
+		}
+
+		/* The counter is only 8 bits.
+		 * Ignore anything in the upper 3 bytes
+		 */
+		cmd_credits &= 0xFF;
+	}
+
+	if (!cmd_credits) {
+		ath10k_err(ar, "bmi communication timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int ath10k_sdio_bmi_get_rx_lookahead(struct ath10k *ar)
+{
+	unsigned long timeout;
+	u32 rx_word = 0;
+	int ret = 0;
+
+	timeout = jiffies + BMI_COMMUNICATION_TIMEOUT_HZ;
+	while ((time_before(jiffies, timeout)) && !rx_word) {
+		ret = ath10k_sdio_read_write_sync(ar,
+						  MBOX_HOST_INT_STATUS_ADDRESS,
+						  (u8 *)&rx_word,
+						  sizeof(rx_word),
+						  HIF_RD_SYNC_BYTE_INC);
+		if (ret) {
+			ath10k_err(ar, "unable to read RX_LOOKAHEAD_VALID\n");
+			return ret;
+		}
+
+		 /* all we really want is one bit */
+		rx_word &= 1;
+	}
+
+	if (!rx_word) {
+		ath10k_err(ar, "bmi_recv_buf FIFO empty\n");
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+int ath10k_sdio_hif_exchange_bmi_msg(struct ath10k *ar,
+				     void *req, u32 req_len,
+				     void *resp, u32 *resp_len)
+{
+	int ret = 0;
+	u32 addr;
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+
+	ret = ath10k_sdio_bmi_credits(ar);
+	if (ret)
+		return ret;
+
+	/* BMI data is written to the end of the mailbox address space */
+	addr = ar_sdio->mbox_info.htc_addr + ATH10K_HIF_MBOX_WIDTH - req_len;
+
+	ret = ath10k_sdio_read_write_sync(ar, addr, req, req_len,
+					  HIF_WR_SYNC_BYTE_INC);
+	if (ret) {
+		ath10k_err(ar, "unable to send the bmi data to the device\n");
+		return ret;
+	}
+
+	if (!resp || !resp_len)
+		/* No response expected */
+		goto out;
+
+	/* During normal bootup, small reads may be required.
+	 * Rather than issue an HIF Read and then wait as the Target
+	 * adds successive bytes to the FIFO, we wait here until
+	 * we know that response data is available.
+	 *
+	 * This allows us to cleanly timeout on an unexpected
+	 * Target failure rather than risk problems at the HIF level.
+	 * In particular, this avoids SDIO timeouts and possibly garbage
+	 * data on some host controllers.  And on an interconnect
+	 * such as Compact Flash (as well as some SDIO masters) which
+	 * does not provide any indication on data timeout, it avoids
+	 * a potential hang or garbage response.
+	 *
+	 * Synchronization is more difficult for reads larger than the
+	 * size of the MBOX FIFO (128B), because the Target is unable
+	 * to push the 129th byte of data until AFTER the Host posts an
+	 * HIF Read and removes some FIFO data.  So for large reads the
+	 * Host proceeds to post an HIF Read BEFORE all the data is
+	 * actually available to read.  Fortunately, large BMI reads do
+	 * not occur in practice -- they're supported for debug/development.
+	 *
+	 * So Host/Target BMI synchronization is divided into these cases:
+	 *  CASE 1: length < 4
+	 *        Should not happen
+	 *
+	 *  CASE 2: 4 <= length <= 128
+	 *        Wait for first 4 bytes to be in FIFO
+	 *        If CONSERVATIVE_BMI_READ is enabled, also wait for
+	 *        a BMI command credit, which indicates that the ENTIRE
+	 *        response is available in the the FIFO
+	 *
+	 *  CASE 3: length > 128
+	 *        Wait for the first 4 bytes to be in FIFO
+	 *
+	 * For most uses, a small timeout should be sufficient and we will
+	 * usually see a response quickly; but there may be some unusual
+	 * (debug) cases of BMI_EXECUTE where we want an larger timeout.
+	 * For now, we use an unbounded busy loop while waiting for
+	 * BMI_EXECUTE.
+	 *
+	 * If BMI_EXECUTE ever needs to support longer-latency execution,
+	 * especially in production, this code needs to be enhanced to sleep
+	 * and yield.  Also note that BMI_COMMUNICATION_TIMEOUT is currently
+	 * a function of Host processor speed.
+	 */
+	ret = ath10k_sdio_bmi_get_rx_lookahead(ar);
+	if (ret)
+		goto out;
+
+	/* We always read from the start of the mbox address */
+	addr = ar_sdio->mbox_info.htc_addr;
+	ret = ath10k_sdio_read_write_sync(ar, addr, resp, *resp_len,
+					  HIF_RD_SYNC_BYTE_INC);
+	if (ret)
+		ath10k_err(ar, "Unable to read the bmi data from the device: %d\n",
+			   ret);
+
+out:
+	return ret;
+}
+
+static void ath10k_sdio_hif_stop(struct ath10k *ar)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct ath10k_sdio_bus_request *req, *tmp_req;
+
+	ath10k_sdio_irq_disable(ar);
+
+	cancel_work_sync(&ar_sdio->wr_async_work);
+
+	spin_lock_bh(&ar_sdio->wr_async_lock);
+
+	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
+		struct ath10k_htc_ep *ep;
+
+		list_del(&req->list);
+
+		ep = &ar->htc.endpoint[req->eid];
+		ath10k_htc_notify_tx_completion(ep, req->skb);
+		ath10k_sdio_free_bus_req(ar_sdio, req);
+	}
+
+	spin_unlock_bh(&ar_sdio->wr_async_lock);
+}
+
+#ifdef CONFIG_PM
+
+static int ath10k_sdio_hif_suspend(struct ath10k *ar)
+{
+	return -EOPNOTSUPP;
+}
+
+static int ath10k_sdio_hif_resume(struct ath10k *ar)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+
+	switch (ar->state) {
+	case ATH10K_STATE_OFF:
+		ath10k_dbg(ar, ATH10K_DBG_SDIO,
+			   "sdio resume configuring sdio\n");
+
+		/* need to set sdio settings after power is cut from sdio */
+		ath10k_sdio_config(ar_sdio);
+		break;
+
+	case ATH10K_STATE_ON:
+	default:
+		break;
+	}
+
+	return 0;
+}
+#endif
+
+int ath10k_sdio_hif_map_service_to_pipe(struct ath10k *ar, u16 service_id,
+					u8 *ul_pipe, u8 *dl_pipe)
+{
+	int ret = 0, i;
+	bool ep_found = false;
+	enum ath10k_htc_ep_id eid;
+	struct ath10k_htc *htc = &ar->htc;
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	u32 htt_addr, wmi_addr, htt_mbox_size, wmi_mbox_size;
+
+	/* For sdio, we are interested in the mapping between eid
+	 * and pipeid rather than service_id to pipe_id.
+	 * First we find out which eid has been allocated to the
+	 * service...
+	 */
+	for (i = 0; i < ATH10K_HTC_EP_COUNT; i++) {
+		if (htc->endpoint[i].service_id == service_id) {
+			eid = htc->endpoint[i].eid;
+			ep_found = true;
+			break;
+		}
+	}
+
+	if (!ep_found) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Then we create the simplest mapping possible between pipeid
+	 * and eid
+	 */
+	*ul_pipe = *dl_pipe = (u8)eid;
+
+	/* Normally, HTT will use the upper part of the extended
+	 * mailbox address space (ext_info[1].htc_ext_addr) and WMI ctrl
+	 * the lower part (ext_info[0].htc_ext_addr).
+	 * If fw wants swapping of mailbox addresses, the opposite is true.
+	 */
+	if (ar_sdio->swap_mbox) {
+		htt_addr = ar_sdio->mbox_info.ext_info[0].htc_ext_addr;
+		wmi_addr = ar_sdio->mbox_info.ext_info[1].htc_ext_addr;
+		htt_mbox_size = ar_sdio->mbox_info.ext_info[0].htc_ext_sz;
+		wmi_mbox_size = ar_sdio->mbox_info.ext_info[1].htc_ext_sz;
+	} else {
+		htt_addr = ar_sdio->mbox_info.ext_info[1].htc_ext_addr;
+		wmi_addr = ar_sdio->mbox_info.ext_info[0].htc_ext_addr;
+		htt_mbox_size = ar_sdio->mbox_info.ext_info[1].htc_ext_sz;
+		wmi_mbox_size = ar_sdio->mbox_info.ext_info[0].htc_ext_sz;
+	}
+
+	switch (service_id) {
+	case ATH10K_HTC_SVC_ID_RSVD_CTRL:
+		/* HTC ctrl ep mbox address has already been setup in
+		 * ath10k_sdio_hif_start
+		 */
+		break;
+	case ATH10K_HTC_SVC_ID_WMI_CONTROL:
+		ar_sdio->mbox_addr[eid] = wmi_addr;
+		ar_sdio->mbox_size[eid] = wmi_mbox_size;
+		ath10k_dbg(ar, ATH10K_DBG_SDIO,
+			   "WMI ctrl mbox addr = %x, mbox_size = %x\n",
+			   ar_sdio->mbox_addr[eid], ar_sdio->mbox_size[eid]);
+		break;
+	case ATH10K_HTC_SVC_ID_HTT_DATA_MSG:
+		ar_sdio->mbox_addr[eid] = htt_addr;
+		ar_sdio->mbox_size[eid] = htt_mbox_size;
+		ath10k_dbg(ar, ATH10K_DBG_SDIO,
+			   "HTT data mbox addr = %x, mbox_size = %x\n",
+			   ar_sdio->mbox_addr[eid], ar_sdio->mbox_size[eid]);
+		break;
+	default:
+		ath10k_err(ar, "Unsupported service ID: %x\n",
+			   service_id);
+		ret = -EINVAL;
+	}
+
+out:
+	return ret;
+}
+
+void ath10k_sdio_hif_get_default_pipe(struct ath10k *ar,
+				      u8 *ul_pipe, u8 *dl_pipe)
+{
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio hif get default pipe\n");
+
+	/* HTC ctrl ep (SVC id 1) always has eid (and pipe_id in our
+	 * case) == 0
+	 */
+	*ul_pipe = 0;
+	*dl_pipe = 0;
+}
+
+static int ath10k_sdio_hif_fetch_cal_eeprom(struct ath10k *ar, void **data,
+					    size_t *data_len)
+{
+	return -EOPNOTSUPP;
+}
+
+static void ath10k_sdio_write32(struct ath10k *ar, u32 offset, u32 value)
+{
+}
+
+static u32 ath10k_sdio_read32(struct ath10k *ar, u32 offset)
+{
+	return 0;
+}
+
+static void ath10k_sdio_hif_send_complete_check(struct ath10k *ar,
+						u8 pipe, int force)
+{
+}
+
+static u16 ath10k_sdio_hif_get_free_queue_number(struct ath10k *ar, u8 pipe)
+{
+	return 0;
+}
+
+static const struct ath10k_hif_ops ath10k_sdio_hif_ops = {
+	.tx_sg			= ath10k_sdio_hif_tx_sg,
+	.diag_read		= ath10k_sdio_hif_diag_read,
+	.diag_write		= ath10k_sdio_diag_write_mem,
+	.exchange_bmi_msg	= ath10k_sdio_hif_exchange_bmi_msg,
+	.start			= ath10k_sdio_hif_start,
+	.stop			= ath10k_sdio_hif_stop,
+	.map_service_to_pipe	= ath10k_sdio_hif_map_service_to_pipe,
+	.get_default_pipe	= ath10k_sdio_hif_get_default_pipe,
+	.send_complete_check	= ath10k_sdio_hif_send_complete_check,
+	.get_free_queue_number	= ath10k_sdio_hif_get_free_queue_number,
+	.power_up		= ath10k_sdio_hif_power_up,
+	.power_down		= ath10k_sdio_hif_power_down,
+	.read32			= ath10k_sdio_read32,
+	.write32		= ath10k_sdio_write32,
+#ifdef CONFIG_PM
+	.suspend		= ath10k_sdio_hif_suspend,
+	.resume			= ath10k_sdio_hif_resume,
+#endif
+	.fetch_cal_eeprom	= ath10k_sdio_hif_fetch_cal_eeprom,
+};
+
+#ifdef CONFIG_PM_SLEEP
+
+/* Empty handlers so that mmc subsystem doesn't remove us entirely during
+ * suspend. We instead follow cfg80211 suspend/resume handlers.
+ */
+static int ath10k_sdio_pm_suspend(struct device *device)
+{
+	return 0;
+}
+
+static int ath10k_sdio_pm_resume(struct device *device)
+{
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ath10k_sdio_pm_ops, ath10k_sdio_pm_suspend,
+			 ath10k_sdio_pm_resume);
+
+#define ATH10K_SDIO_PM_OPS (&ath10k_sdio_pm_ops)
+
+#else
+
+#define ATH10K_SDIO_PM_OPS NULL
+
+#endif /* CONFIG_PM_SLEEP */
+
+static int ath10k_sdio_probe(struct sdio_func *func,
+			     const struct sdio_device_id *id)
+{
+	int ret;
+	struct ath10k_sdio *ar_sdio;
+	struct ath10k *ar;
+	int i;
+	enum ath10k_hw_rev hw_rev;
+
+	hw_rev = ATH10K_HW_QCA6174;
+
+	ar = ath10k_core_create(sizeof(*ar_sdio), &func->dev, ATH10K_BUS_SDIO,
+				hw_rev, &ath10k_sdio_hif_ops);
+	if (!ar) {
+		dev_err(&func->dev, "failed to allocate core\n");
+		return -ENOMEM;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT,
+		   "sdio new func %d vendor 0x%x device 0x%x block 0x%x/0x%x\n",
+		   func->num, func->vendor, func->device,
+		   func->max_blksize, func->cur_blksize);
+
+	ar_sdio = ath10k_sdio_priv(ar);
+
+	ar_sdio->dma_buffer = kzalloc(ATH10K_HIF_DMA_BUFFER_SIZE, GFP_KERNEL);
+	if (!ar_sdio->dma_buffer) {
+		ret = -ENOMEM;
+		goto err_core_destroy;
+	}
+
+	ar_sdio->func = func;
+	sdio_set_drvdata(func, ar_sdio);
+
+	ar_sdio->is_disabled = true;
+	ar_sdio->ar = ar;
+
+	spin_lock_init(&ar_sdio->lock);
+	spin_lock_init(&ar_sdio->wr_async_lock);
+	spin_lock_init(&ar_sdio->irq_data.lock);
+	mutex_init(&ar_sdio->dma_buffer_mutex);
+
+	INIT_LIST_HEAD(&ar_sdio->bus_req_freeq);
+	INIT_LIST_HEAD(&ar_sdio->wr_asyncq);
+
+	INIT_WORK(&ar_sdio->wr_async_work, ath10k_sdio_write_async_work);
+	ar_sdio->workqueue = create_singlethread_workqueue("ath10k_sdio_wq");
+	if (!ar_sdio->workqueue)
+		goto err;
+
+	init_waitqueue_head(&ar_sdio->irq_wq);
+
+	for (i = 0; i < ATH10K_SDIO_BUS_REQUEST_MAX_NUM; i++)
+		ath10k_sdio_free_bus_req(ar_sdio, &ar_sdio->bus_req[i]);
+
+	ar->dev_id = id->device;
+	ar->id.vendor = id->vendor;
+	ar->id.device = id->device;
+
+	ath10k_sdio_set_mbox_info(ar_sdio);
+
+	ret = ath10k_sdio_config(ar_sdio);
+	if (ret) {
+		ath10k_err(ar, "Failed to config sdio: %d\n", ret);
+		goto err;
+	}
+
+	ret = ath10k_core_register(ar, 0/*chip_id is not applicaple for SDIO*/);
+	if (ret) {
+		ath10k_err(ar, "failed to register driver core: %d\n", ret);
+		goto err;
+	}
+
+	return ret;
+
+err:
+	kfree(ar_sdio->dma_buffer);
+err_core_destroy:
+	ath10k_core_destroy(ar);
+
+	return ret;
+}
+
+static void ath10k_sdio_remove(struct sdio_func *func)
+{
+	struct ath10k_sdio *ar_sdio;
+	struct ath10k *ar;
+
+	ar_sdio = sdio_get_drvdata(func);
+	ar = ar_sdio->ar;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT,
+		   "sdio removed func %d vendor 0x%x device 0x%x\n",
+		   func->num, func->vendor, func->device);
+
+	(void)ath10k_sdio_hif_disable_intrs(ar_sdio);
+	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[] = {
+	{SDIO_DEVICE(ATH10K_MANUFACTURER_CODE,
+		     (ATH10K_MANUFACTURER_ID_AR6005_BASE | 0xA))},
+	{},
+};
+
+MODULE_DEVICE_TABLE(sdio, ath10k_sdio_devices);
+
+static struct sdio_driver ath10k_sdio_driver = {
+	.name = "ath10k_sdio",
+	.id_table = ath10k_sdio_devices,
+	.probe = ath10k_sdio_probe,
+	.remove = ath10k_sdio_remove,
+	.drv.pm = ATH10K_SDIO_PM_OPS,
+};
+
+static int __init ath10k_sdio_init(void)
+{
+	int ret;
+
+	ret = sdio_register_driver(&ath10k_sdio_driver);
+	if (ret)
+		pr_err("sdio driver registration failed: %d\n", ret);
+
+	return ret;
+}
+
+static void __exit ath10k_sdio_exit(void)
+{
+	sdio_unregister_driver(&ath10k_sdio_driver);
+}
+
+module_init(ath10k_sdio_init);
+module_exit(ath10k_sdio_exit);
+
+MODULE_AUTHOR("Qualcomm Atheros");
+MODULE_DESCRIPTION("Driver support for Qualcomm Atheros 802.11ac WLAN SDIO devices");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
new file mode 100644
index 0000000..b5887c1
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -0,0 +1,276 @@
+/*
+ * Copyright (c) 2004-2011 Atheros Communications Inc.
+ * Copyright (c) 2011-2012 Qualcomm Atheros, Inc.
+ * Copyright (c) 2016 Kapsch Trafficcom AB
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef _SDIO_H_
+#define _SDIO_H_
+
+#define ATH10K_HIF_MBOX_BLOCK_SIZE              256
+#define ATH10K_MANUFACTURER_ID_AR6005_BASE      0x500
+
+#define ATH10K_MANUFACTURER_ID_REV_MASK         0x00FF
+#define ATH10K_MANUFACTURER_CODE                0x271 /* Atheros */
+
+#define ATH10K_HIF_DMA_BUFFER_SIZE              (32 * 1024)
+#define ATH10K_SDIO_MAX_BUFFER_SIZE             4096 /*Unsure of this constant*/
+
+/* Mailbox address in SDIO address space */
+#define ATH10K_HIF_MBOX_BASE_ADDR               0x1000
+#define ATH10K_HIF_MBOX_WIDTH                   0x800
+
+#define ATH10K_HIF_MBOX_TOT_WIDTH \
+	(ATH10K_HIF_MBOX_NUM_MAX * ATH10K_HIF_MBOX_WIDTH)
+
+#define ATH10K_HIF_MBOX0_EXT_BASE_ADDR          0x5000
+#define ATH10K_HIF_MBOX0_EXT_WIDTH              (36 * 1024)
+#define ATH10K_HIF_MBOX0_EXT_WIDTH_ROME_2_0     (56 * 1024)
+#define ATH10K_HIF_MBOX1_EXT_WIDTH              (36 * 1024)
+#define ATH10K_HIF_MBOX_DUMMY_SPACE_SIZE        (2 * 1024)
+
+#define ATH10K_HTC_MBOX_MAX_PAYLOAD_LENGTH \
+	(ATH10K_SDIO_MAX_BUFFER_SIZE - sizeof(struct ath10k_htc_hdr))
+
+#define ATH10K_HIF_MBOX_NUM_MAX                 4
+#define ATH10K_SDIO_BUS_REQUEST_MAX_NUM         64
+
+#define ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ (100 * HZ)
+
+/* HTC runs over mailbox 0 */
+#define ATH10K_HTC_MAILBOX                      0
+
+/* GMBOX addresses */
+#define ATH10K_HIF_GMBOX_BASE_ADDR              0x7000
+#define ATH10K_HIF_GMBOX_WIDTH                  0x4000
+
+/* interrupt mode register */
+#define CCCR_SDIO_IRQ_MODE_REG                  0xF0
+#define CCCR_SDIO_IRQ_MODE_REG_SDIO3            0x16
+
+#define CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR   0xF2
+
+#define CCCR_SDIO_DRIVER_STRENGTH_ENABLE_A      0x02
+#define CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C      0x04
+#define CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D      0x08
+
+#define CCCR_SDIO_ASYNC_INT_DELAY_ADDRESS       0xF0
+#define CCCR_SDIO_ASYNC_INT_DELAY_LSB           0x06
+#define CCCR_SDIO_ASYNC_INT_DELAY_MASK          0xC0
+
+/* mode to enable special 4-bit interrupt assertion without clock */
+#define SDIO_IRQ_MODE_ASYNC_4BIT_IRQ            BIT(0)
+#define SDIO_IRQ_MODE_ASYNC_4BIT_IRQ_SDIO3      BIT(1)
+
+#define ATH10K_SDIO_TARGET_DEBUG_INTR_MASK      0x01
+
+/* The theoretical maximum number of RX messages that can be fetched
+ * from the mbox interrupt handler in one loop is derived in the following
+ * way:
+ *
+ * Let's assume that each packet in a bundle of the maximum bundle size
+ * (HTC_HOST_MAX_MSG_PER_BUNDLE) has the HTC header bundle count set
+ * to the maximum value (HTC_HOST_MAX_MSG_PER_BUNDLE).
+ *
+ * in this case the driver must allocate
+ * (HTC_HOST_MAX_MSG_PER_BUNDLE * HTC_HOST_MAX_MSG_PER_BUNDLE) skb's.
+ */
+#define ATH10K_SDIO_MAX_RX_MSGS \
+	(HTC_HOST_MAX_MSG_PER_BUNDLE * HTC_HOST_MAX_MSG_PER_BUNDLE)
+
+struct ath10k_sdio_bus_request {
+	struct list_head list;
+
+	/* sdio address */
+	u32 address;
+	u32 request;
+	size_t len;
+	struct sk_buff *skb;
+	enum ath10k_htc_ep_id eid;
+	int status;
+};
+
+struct ath10k_sdio_rx_data {
+	struct sk_buff *skb;
+	size_t alloc_len;
+	size_t act_len;
+	enum ath10k_htc_ep_id eid;
+	bool part_of_bundle;
+	bool last_in_bundle;
+	int status;
+};
+
+/* direction of transfer (read/write) */
+#define HIF_READ                    0x00000001
+#define HIF_WRITE                   0x00000002
+#define HIF_DIR_MASK                (HIF_READ | HIF_WRITE)
+
+/*     emode - This indicates the whether the command is to be executed in a
+ *             blocking or non-blocking fashion (HIF_SYNCHRONOUS/
+ *             HIF_ASYNCHRONOUS). The read/write data paths in HTC have been
+ *             implemented using the asynchronous mode allowing the the bus
+ *             driver to indicate the completion of operation through the
+ *             registered callback routine. The requirement primarily comes
+ *             from the contexts these operations get called from (a driver's
+ *             transmit context or the ISR context in case of receive).
+ *             Support for both of these modes is essential.
+ */
+#define HIF_SYNCHRONOUS             0x00000010
+#define HIF_ASYNCHRONOUS            0x00000020
+#define HIF_EMODE_MASK              (HIF_SYNCHRONOUS | HIF_ASYNCHRONOUS)
+
+/*     dmode - An interface may support different kinds of commands based on
+ *             the tradeoff between the amount of data it can carry and the
+ *             setup time. Byte and Block modes are supported (HIF_BYTE_BASIS/
+ *             HIF_BLOCK_BASIS). In case of latter, the data is rounded off
+ *             to the nearest block size by padding. The size of the block is
+ *             configurable at compile time using the HIF_BLOCK_SIZE and is
+ *             negotiated with the target during initialization after the
+ *             ATH10K interrupts are enabled.
+ */
+#define HIF_BYTE_BASIS              0x00000040
+#define HIF_BLOCK_BASIS             0x00000080
+#define HIF_DMODE_MASK              (HIF_BYTE_BASIS | HIF_BLOCK_BASIS)
+
+/*     amode - This indicates if the address has to be incremented on ATH10K
+ *             after every read/write operation (HIF?FIXED_ADDRESS/
+ *             HIF_INCREMENTAL_ADDRESS).
+ */
+#define HIF_FIXED_ADDRESS           0x00000100
+#define HIF_INCREMENTAL_ADDRESS     0x00000200
+#define HIF_AMODE_MASK              (HIF_FIXED_ADDRESS | HIF_INCREMENTAL_ADDRESS)
+
+#define HIF_WR_ASYNC_BYTE_INC					\
+	(HIF_WRITE | HIF_ASYNCHRONOUS |				\
+	 HIF_BYTE_BASIS | HIF_INCREMENTAL_ADDRESS)
+
+#define HIF_WR_ASYNC_BLOCK_INC					\
+	(HIF_WRITE | HIF_ASYNCHRONOUS |				\
+	 HIF_BLOCK_BASIS | HIF_INCREMENTAL_ADDRESS)
+
+#define HIF_WR_SYNC_BYTE_FIX					\
+	(HIF_WRITE | HIF_SYNCHRONOUS |				\
+	 HIF_BYTE_BASIS | HIF_FIXED_ADDRESS)
+
+#define HIF_WR_SYNC_BYTE_INC					\
+	(HIF_WRITE | HIF_SYNCHRONOUS |				\
+	 HIF_BYTE_BASIS | HIF_INCREMENTAL_ADDRESS)
+
+#define HIF_WR_SYNC_BLOCK_INC					\
+	(HIF_WRITE | HIF_SYNCHRONOUS |				\
+	 HIF_BLOCK_BASIS | HIF_INCREMENTAL_ADDRESS)
+
+#define HIF_RD_SYNC_BYTE_INC						\
+	(HIF_READ | HIF_SYNCHRONOUS |					\
+	 HIF_BYTE_BASIS | HIF_INCREMENTAL_ADDRESS)
+
+#define HIF_RD_SYNC_BYTE_FIX						\
+	(HIF_READ | HIF_SYNCHRONOUS |					\
+	 HIF_BYTE_BASIS | HIF_FIXED_ADDRESS)
+
+#define HIF_RD_ASYNC_BLOCK_FIX						\
+	(HIF_READ | HIF_ASYNCHRONOUS |					\
+	 HIF_BLOCK_BASIS | HIF_FIXED_ADDRESS)
+
+#define HIF_RD_SYNC_BLOCK_FIX						\
+	(HIF_READ | HIF_SYNCHRONOUS |					\
+	 HIF_BLOCK_BASIS | HIF_FIXED_ADDRESS)
+
+struct ath10k_sdio_irq_proc_registers {
+	u8 host_int_status;
+	u8 cpu_int_status;
+	u8 error_int_status;
+	u8 counter_int_status;
+	u8 mbox_frame;
+	u8 rx_lookahead_valid;
+	u8 host_int_status2;
+	u8 gmbox_rx_avail;
+	__le32 rx_lookahead[2];
+	__le32 rx_gmbox_lookahead_alias[2];
+};
+
+struct ath10k_sdio_irq_enable_reg {
+	u8 int_status_en;
+	u8 cpu_int_status_en;
+	u8 err_int_status_en;
+	u8 cntr_int_status_en;
+};
+
+struct ath10k_sdio_irq_data {
+	/* protects irq_proc_reg and irq_en_reg below */
+	spinlock_t lock;
+	struct ath10k_sdio_irq_proc_registers irq_proc_reg;
+	struct ath10k_sdio_irq_enable_reg irq_en_reg;
+};
+
+struct ath10k_mbox_ext_info {
+	u32 htc_ext_addr;
+	u32 htc_ext_sz;
+};
+
+struct ath10k_mbox_info {
+	u32 htc_addr;
+	struct ath10k_mbox_ext_info ext_info[2];
+	u32 block_size;
+	u32 block_mask;
+	u32 gmbox_addr;
+	u32 gmbox_sz;
+};
+
+struct ath10k_sdio {
+	struct sdio_func *func;
+
+	struct ath10k_mbox_info mbox_info;
+	bool swap_mbox;
+	u32 mbox_addr[ATH10K_HTC_EP_COUNT];
+	u32 mbox_size[ATH10K_HTC_EP_COUNT];
+
+	/* available bus requests */
+	struct ath10k_sdio_bus_request bus_req[ATH10K_SDIO_BUS_REQUEST_MAX_NUM];
+	/* free list of bus requests */
+	struct list_head bus_req_freeq;
+	/* protects access to bus_req_freeq */
+	spinlock_t lock;
+
+	struct ath10k_sdio_rx_data rx_pkts[ATH10K_SDIO_MAX_RX_MSGS];
+	size_t n_rx_pkts;
+
+	struct ath10k *ar;
+	struct ath10k_sdio_irq_data irq_data;
+
+	u8 *dma_buffer;
+
+	/* protects access to dma_buffer */
+	struct mutex dma_buffer_mutex;
+
+	atomic_t irq_handling;
+	atomic_t stopping;
+	wait_queue_head_t irq_wq;
+
+	bool is_disabled;
+
+	struct workqueue_struct *workqueue;
+	struct work_struct wr_async_work;
+	struct list_head wr_asyncq;
+	/* protects access to wr_asyncq */
+	spinlock_t wr_async_lock;
+};
+
+static inline struct ath10k_sdio *ath10k_sdio_priv(struct ath10k *ar)
+{
+	return (struct ath10k_sdio *)ar->drv_priv;
+}
+
+#endif
-- 
1.7.9.5

Re: [RFC v2 11/11] ath10k: Added sdio support

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>

While testing this I noticed few new warnings:

drivers/net/wireless/ath/ath10k/sdio.c: In function ath10k_sdio_probe:
drivers/net/wireless/ath/ath10k/sdio.c:1723:6: warning: 'ret' may be used u=
ninitialized in this function [-Wuninitialized]
drivers/net/wireless/ath/ath10k/sdio.c:375:5: warning: symbol 'ath10k_sdio_=
mbox_rxmsg_pending_handler' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1018:5: warning: symbol 'ath10k_sdio=
_hif_tx_sg' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1415:5: warning: symbol 'ath10k_sdio=
_hif_exchange_bmi_msg' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1555:5: warning: symbol 'ath10k_sdio=
_hif_map_service_to_pipe' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/sdio.c:1635:6: warning: symbol 'ath10k_sdio=
_hif_get_default_pipe' was not declared. Should it be static?
drivers/net/wireless/ath/ath10k/htc.c:265: line over 90 characters
drivers/net/wireless/ath/ath10k/htc.c:355: line over 90 characters

--=20
Kalle Valo=

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> Code refactorization:
>
> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
> to ath10k_htc_control_rx_complete.
>
> This eases the implementation of SDIO/mbox significantly since
> the ep_rx_complete cb is invoked directly from the SDIO/mbox
> hif layer.
>
> Since the ath10k_htc_control_rx_complete already is present
> (only containing a warning message) there is no reason for not
> using it (instead of having a special case for ep 0 in
> ath10k_htc_rx_completion_handler).
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>

I tested this on QCA988X PCI board just to see if there are any
regressions. It crashes immediately during module load, every time, and
bisected that the crashing starts on this patch:

[ 1239.715325] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 irq_mod=
e 0 reset_mode 0
[ 1239.885125] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/pre=
-cal-pci-0000:02:00.0.bin failed with error -2
[ 1239.885260] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/cal=
-pci-0000:02:00.0.bin failed with error -2
[ 1239.885687] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016c chi=
p_id 0x043202ff sub 0000:0000
[ 1239.885699] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1=
 dfs 1 testmode 1
[ 1239.885899] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 api 5 f=
eatures no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
[ 1239.941836] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/QCA=
988X/hw2.0/board-2.bin failed with error -2
[ 1239.941993] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A crc32 b=
ebc7c08
[ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at   (=
null)
[ 1241.136738] IP: [<  (null)>]   (null)
[ 1241.136759] *pdpt =3D 0000000000000000 *pde =3D f0002a55f0002a55 [ 1241.=
136781]=20
[ 1241.136793] Oops: 0010 [#1] SMP

What's odd is that when I added some printks on my own and enabled both
boot and htc debug levels it doesn't crash anymore. After everything
works normally after that, I can start AP mode and connect to it. Is it
a race somewhere?

--=20
Kalle Valo=

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Michal Kazior]

On 13 December 2016 at 14:44, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>
>> Code refactorization:
>>
>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>> to ath10k_htc_control_rx_complete.
>>
>> This eases the implementation of SDIO/mbox significantly since
>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>> hif layer.
>>
>> Since the ath10k_htc_control_rx_complete already is present
>> (only containing a warning message) there is no reason for not
>> using it (instead of having a special case for ep 0 in
>> ath10k_htc_rx_completion_handler).
>>
>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
>
> I tested this on QCA988X PCI board just to see if there are any
> regressions. It crashes immediately during module load, every time, and
> bisected that the crashing starts on this patch:
>
> [ 1239.715325] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 irq_m=
ode 0 reset_mode 0
> [ 1239.885125] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/p=
re-cal-pci-0000:02:00.0.bin failed with error -2
> [ 1239.885260] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/c=
al-pci-0000:02:00.0.bin failed with error -2
> [ 1239.885687] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016c c=
hip_id 0x043202ff sub 0000:0000
> [ 1239.885699] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing=
 1 dfs 1 testmode 1
> [ 1239.885899] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 api 5=
 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
> [ 1239.941836] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/Q=
CA988X/hw2.0/board-2.bin failed with error -2
> [ 1239.941993] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A crc32=
 bebc7c08
> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at  =
 (null)
> [ 1241.136738] IP: [<  (null)>]   (null)
> [ 1241.136759] *pdpt =3D 0000000000000000 *pde =3D f0002a55f0002a55 [ 124=
1.136781]
> [ 1241.136793] Oops: 0010 [#1] SMP
>
> What's odd is that when I added some printks on my own and enabled both
> boot and htc debug levels it doesn't crash anymore. After everything
> works normally after that, I can start AP mode and connect to it. Is it
> a race somewhere?

Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
is set in htc_wait_target() [changed patch 4, but still too late].

ep_rx_complete must be set prior to calling hif_start(). You probably
crash on end of ath10k_htc_rx_completion_handler() when trying to call
ep->ep_ops.ep_rx_complete(ar, skb).


Micha=C5=82

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Valo, Kalle]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 13 December 2016 at 14:44, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>
>>> Code refactorization:
>>>
>>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>>> to ath10k_htc_control_rx_complete.
>>>
>>> This eases the implementation of SDIO/mbox significantly since
>>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>>> hif layer.
>>>
>>> Since the ath10k_htc_control_rx_complete already is present
>>> (only containing a warning message) there is no reason for not
>>> using it (instead of having a special case for ep 0 in
>>> ath10k_htc_rx_completion_handler).
>>>
>>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
>>
>> I tested this on QCA988X PCI board just to see if there are any
>> regressions. It crashes immediately during module load, every time, and
>> bisected that the crashing starts on this patch:
>>
>> [ 1239.715325] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 irq_=
mode 0 reset_mode 0
>> [ 1239.885125] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/=
pre-cal-pci-0000:02:00.0.bin failed with error -2
>> [ 1239.885260] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/=
cal-pci-0000:02:00.0.bin failed with error -2
>> [ 1239.885687] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016c =
chip_id 0x043202ff sub 0000:0000
>> [ 1239.885699] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracin=
g 1 dfs 1 testmode 1
>> [ 1239.885899] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 api =
5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
>> [ 1239.941836] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/=
QCA988X/hw2.0/board-2.bin failed with error -2
>> [ 1239.941993] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A crc3=
2 bebc7c08
>> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at =
  (null)
>> [ 1241.136738] IP: [<  (null)>]   (null)
>> [ 1241.136759] *pdpt =3D 0000000000000000 *pde =3D f0002a55f0002a55 [ 12=
41.136781]
>> [ 1241.136793] Oops: 0010 [#1] SMP
>>
>> What's odd is that when I added some printks on my own and enabled both
>> boot and htc debug levels it doesn't crash anymore. After everything
>> works normally after that, I can start AP mode and connect to it. Is it
>> a race somewhere?
>
> Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
> is set in htc_wait_target() [changed patch 4, but still too late].
>
> ep_rx_complete must be set prior to calling hif_start(). You probably
> crash on end of ath10k_htc_rx_completion_handler() when trying to call
> ep->ep_ops.ep_rx_complete(ar, skb).

Yeah, just checked and ep->ep_ops.ep_rx_complete is NULL at the end of
ath10k_htc_rx_completion_handler().

--=20
Kalle Valo=

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Erik Stromdahl]

On 12/13/2016 06:26 PM, Valo, Kalle wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
> 
>> On 13 December 2016 at 14:44, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>>
>>>> Code refactorization:
>>>>
>>>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>>>> to ath10k_htc_control_rx_complete.
>>>>
>>>> This eases the implementation of SDIO/mbox significantly since
>>>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>>>> hif layer.
>>>>
>>>> Since the ath10k_htc_control_rx_complete already is present
>>>> (only containing a warning message) there is no reason for not
>>>> using it (instead of having a special case for ep 0 in
>>>> ath10k_htc_rx_completion_handler).
>>>>
>>>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
>>>
>>> I tested this on QCA988X PCI board just to see if there are any
>>> regressions. It crashes immediately during module load, every time, and
>>> bisected that the crashing starts on this patch:
>>>
>>> [ 1239.715325] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 reset_mode 0
>>> [ 1239.885125] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/pre-cal-pci-0000:02:00.0.bin failed with error -2
>>> [ 1239.885260] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/cal-pci-0000:02:00.0.bin failed with error -2
>>> [ 1239.885687] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016c chip_id 0x043202ff sub 0000:0000
>>> [ 1239.885699] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1
>>> [ 1239.885899] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 api 5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
>>> [ 1239.941836] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
>>> [ 1239.941993] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A crc32 bebc7c08
>>> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at   (null)
>>> [ 1241.136738] IP: [<  (null)>]   (null)
>>> [ 1241.136759] *pdpt = 0000000000000000 *pde = f0002a55f0002a55 [ 1241.136781]
>>> [ 1241.136793] Oops: 0010 [#1] SMP
>>>
>>> What's odd is that when I added some printks on my own and enabled both
>>> boot and htc debug levels it doesn't crash anymore. After everything
>>> works normally after that, I can start AP mode and connect to it. Is it
>>> a race somewhere?
>>
>> Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
>> is set in htc_wait_target() [changed patch 4, but still too late].
>>
>> ep_rx_complete must be set prior to calling hif_start(). You probably
>> crash on end of ath10k_htc_rx_completion_handler() when trying to call
>> ep->ep_ops.ep_rx_complete(ar, skb).
> 
> Yeah, just checked and ep->ep_ops.ep_rx_complete is NULL at the end of
> ath10k_htc_rx_completion_handler().
> 
It is indeed correct as Michal points out, there is a risk that the
first HTC control message (typically an HTC ready message) is received
before the HTC control endpoint is connected.

I have experienced a similar race with my SDIO implementation as well.
In this case I did solve the issue by enabling HIF target interrupts
after the HTC control endpoint was connected. I am not sure however if
this is the most elegant way to solve this problem.

My SDIO target won't send the HTC ready message before this is done.
The fix essentially consists of moving the ..._irg_enable call from
hif_start into another hif op.

I have made a few updates since I submitted the original RFC and created
a repo on github:

https://github.com/erstrom/linux-ath

I have a bunch of branches that are all based on the tags on the ath master.

As of this moment the latest version is:

ath-201612131156-ath10k-sdio

This branch contains the original RFC patches plus some addons/fixes.

In the above mentioned branch there are a few commits related to this
race condition. Perhaps you can have a look at them?

The commits are:
821672913328cf737c9616786dc28d2e4e8a4a90
dd7fcf0a1f78e68876d14f90c12bd37f3a700ad7
7434b7b40875bd08a3a48a437ba50afed7754931

Perhaps this approach can work with PCIe as well?

/Erik

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Michal Kazior]

On 13 December 2016 at 19:37, Erik Stromdahl <erik.stromdahl@gmail.com> wro=
te:
>
>
> On 12/13/2016 06:26 PM, Valo, Kalle wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> On 13 December 2016 at 14:44, Valo, Kalle <kvalo@qca.qualcomm.com> wrot=
e:
>>>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>>>
>>>>> Code refactorization:
>>>>>
>>>>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>>>>> to ath10k_htc_control_rx_complete.
>>>>>
>>>>> This eases the implementation of SDIO/mbox significantly since
>>>>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>>>>> hif layer.
>>>>>
>>>>> Since the ath10k_htc_control_rx_complete already is present
>>>>> (only containing a warning message) there is no reason for not
>>>>> using it (instead of having a special case for ep 0 in
>>>>> ath10k_htc_rx_completion_handler).
>>>>>
>>>>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
>>>>
>>>> I tested this on QCA988X PCI board just to see if there are any
>>>> regressions. It crashes immediately during module load, every time, an=
d
>>>> bisected that the crashing starts on this patch:
>>>>
>>>> [ 1239.715325] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 ir=
q_mode 0 reset_mode 0
>>>> [ 1239.885125] ath10k_pci 0000:02:00.0: Direct firmware load for ath10=
k/pre-cal-pci-0000:02:00.0.bin failed with error -2
>>>> [ 1239.885260] ath10k_pci 0000:02:00.0: Direct firmware load for ath10=
k/cal-pci-0000:02:00.0.bin failed with error -2
>>>> [ 1239.885687] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016=
c chip_id 0x043202ff sub 0000:0000
>>>> [ 1239.885699] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 trac=
ing 1 dfs 1 testmode 1
>>>> [ 1239.885899] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 ap=
i 5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
>>>> [ 1239.941836] ath10k_pci 0000:02:00.0: Direct firmware load for ath10=
k/QCA988X/hw2.0/board-2.bin failed with error -2
>>>> [ 1239.941993] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A cr=
c32 bebc7c08
>>>> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference a=
t   (null)
>>>> [ 1241.136738] IP: [<  (null)>]   (null)
>>>> [ 1241.136759] *pdpt =3D 0000000000000000 *pde =3D f0002a55f0002a55 [ =
1241.136781]
>>>> [ 1241.136793] Oops: 0010 [#1] SMP
>>>>
>>>> What's odd is that when I added some printks on my own and enabled bot=
h
>>>> boot and htc debug levels it doesn't crash anymore. After everything
>>>> works normally after that, I can start AP mode and connect to it. Is i=
t
>>>> a race somewhere?
>>>
>>> Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
>>> is set in htc_wait_target() [changed patch 4, but still too late].
>>>
>>> ep_rx_complete must be set prior to calling hif_start(). You probably
>>> crash on end of ath10k_htc_rx_completion_handler() when trying to call
>>> ep->ep_ops.ep_rx_complete(ar, skb).
>>
>> Yeah, just checked and ep->ep_ops.ep_rx_complete is NULL at the end of
>> ath10k_htc_rx_completion_handler().
>>
> It is indeed correct as Michal points out, there is a risk that the
> first HTC control message (typically an HTC ready message) is received
> before the HTC control endpoint is connected.
>
> I have experienced a similar race with my SDIO implementation as well.
> In this case I did solve the issue by enabling HIF target interrupts
> after the HTC control endpoint was connected. I am not sure however if
> this is the most elegant way to solve this problem.
>
> My SDIO target won't send the HTC ready message before this is done.
> The fix essentially consists of moving the ..._irg_enable call from
> hif_start into another hif op.

It makes more sense to move ep_rx_complete setup/assignment before
hif_start(). This assignment should be done very early as there is
nothing to change/override for this endpoint during operation, is
there? It's known what it needs to store very early on.


> I have made a few updates since I submitted the original RFC and created
> a repo on github:
>
> https://github.com/erstrom/linux-ath
>
> I have a bunch of branches that are all based on the tags on the ath mast=
er.
>
> As of this moment the latest version is:
>
> ath-201612131156-ath10k-sdio
>
> This branch contains the original RFC patches plus some addons/fixes.
>
> In the above mentioned branch there are a few commits related to this
> race condition. Perhaps you can have a look at them?
>
> The commits are:
> 821672913328cf737c9616786dc28d2e4e8a4a90

I would avoid if(bus=3D=3Dxx) checks.


> dd7fcf0a1f78e68876d14f90c12bd37f3a700ad7
> 7434b7b40875bd08a3a48a437ba50afed7754931
>
> Perhaps this approach can work with PCIe as well?

I think I did contemplate the unmask/start distinction at some point
but I didn't go with it for some reason I can't recall now.


Micha=C5=82

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> I have made a few updates since I submitted the original RFC and created
> a repo on github:
>
> https://github.com/erstrom/linux-ath
>
> I have a bunch of branches that are all based on the tags on the ath mast=
er.
>
> As of this moment the latest version is:
>
> ath-201612131156-ath10k-sdio
>
> This branch contains the original RFC patches plus some addons/fixes.

Good, this makes it easier to follow the development. So what's the
current status with this branch? What works and what doesn't?

Especially I'm interested about the state of the HTT high latency
support. How much work is to add that? It would also make it easier to
add USB support to ath10k.

--=20
Kalle Valo=

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Valo, Kalle]

Michal Kazior <michal.kazior@tieto.com> writes:

>> I have made a few updates since I submitted the original RFC and created
>> a repo on github:
>>
>> https://github.com/erstrom/linux-ath
>>
>> I have a bunch of branches that are all based on the tags on the ath mas=
ter.
>>
>> As of this moment the latest version is:
>>
>> ath-201612131156-ath10k-sdio
>>
>> This branch contains the original RFC patches plus some addons/fixes.
>>
>> In the above mentioned branch there are a few commits related to this
>> race condition. Perhaps you can have a look at them?
>>
>> The commits are:
>> 821672913328cf737c9616786dc28d2e4e8a4a90
>
> I would avoid if(bus=3D=3Dxx) checks.

Very much agreed. For example to enable HTT high latency support you
could add an enum to ath10k_core_create() with values for both high and
low latency. This way sdio.c and pci.c can enable the correct mode
during initialisation.

--=20
Kalle Valo=

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Erik Stromdahl]

On 12/14/2016 02:46 PM, Valo, Kalle wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> 
>> I have made a few updates since I submitted the original RFC and created
>> a repo on github:
>>
>> https://github.com/erstrom/linux-ath
>>
>> I have a bunch of branches that are all based on the tags on the ath master.
>>
>> As of this moment the latest version is:
>>
>> ath-201612131156-ath10k-sdio
>>
>> This branch contains the original RFC patches plus some addons/fixes.
> 
> Good, this makes it easier to follow the development. So what's the
> current status with this branch? What works and what doesn't?
> 
Well, everything in there has been tested (limited though, as you
yourself experienced), but it is not complete. I still have some other
patches that need some more refurbishing before I can push them.

I will push those patches that I consider "good enough" for publish to
this repo. Most likely I will rewrite/squash etc. some of it before
submitting a new RFC series.

So, there are still some additional stuff that needs to be added. In my
case I have a series of patches related to OCB (Outside the Context of a
BSS) mode of operation (since the chipset I am using is intended for
this purpose). These patches are still not complete.

Other SDIO 11ac chipsets might just need an item in the struct
ath10k_hw_params array together with some firmware files.

> Especially I'm interested about the state of the HTT high latency
> support. How much work is to add that? It would also make it easier to
> add USB support to ath10k.
> 

I actually have some patches regarding this, but they have not been
tested at all since I have not yet managed to fully configure my SDIO
chip properly so far. I must finish the OCB code I mentioned earlier and
fix another really annoying issue with a missing HTT version response
(sometimes target won't respond to the HTT version request).

Then, hopefully, I can make some TX and RX tests.

I think the HTT TX part is fairly straight forward. But the RX part is a
little bit more tricky since I am not really sure about how to interface
mac80211 in the RX path.

My work flow goes like this:

As soon as there is a new tag on the ath.git master, I rebase my stuff
on top of it and push a new branch to my repo.

I will continuously push updates to the latest branch (the branch based
on the latest ath tag).

Older branches will not be maintained.

/Erik

Re: [RFC v2 11/11] ath10k: Added sdio support

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/Kconfig  |    6 +
>  drivers/net/wireless/ath/ath10k/Makefile |    3 +
>  drivers/net/wireless/ath/ath10k/sdio.c   | 1860 ++++++++++++++++++++++++=
++++++
>  drivers/net/wireless/ath/ath10k/sdio.h   |  276 +++++
>  4 files changed, 2145 insertions(+)
>  create mode 100644 drivers/net/wireless/ath/ath10k/sdio.c
>  create mode 100644 drivers/net/wireless/ath/ath10k/sdio.h

AFAIK the sdio firmware binary is different from pci and usb. And as all
the firmware images need to coexist in the same repository I suspect we
can't continue to use firmware-N.bin for both pcie and sdio (and in
future usb). So should we somehow rename the sdio firmware filename to
something else, like firmware-sdio-N.bin?

--=20
Kalle Valo=

Re: [RFC v2 11/11] ath10k: Added sdio support

[Erik Stromdahl]

On 12/15/2016 05:40 PM, Valo, Kalle wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> 
>> Initial HIF sdio/mailbox implementation.
>>
>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/Kconfig  |    6 +
>>  drivers/net/wireless/ath/ath10k/Makefile |    3 +
>>  drivers/net/wireless/ath/ath10k/sdio.c   | 1860 ++++++++++++++++++++++++++++++
>>  drivers/net/wireless/ath/ath10k/sdio.h   |  276 +++++
>>  4 files changed, 2145 insertions(+)
>>  create mode 100644 drivers/net/wireless/ath/ath10k/sdio.c
>>  create mode 100644 drivers/net/wireless/ath/ath10k/sdio.h
> 
> AFAIK the sdio firmware binary is different from pci and usb. And as all
> the firmware images need to coexist in the same repository I suspect we
> can't continue to use firmware-N.bin for both pcie and sdio (and in
> future usb). So should we somehow rename the sdio firmware filename to
> something else, like firmware-sdio-N.bin?
> 
I suppose you are right. I would be surprised if the firmware images
were the same for sdio, usb and pci. Hopefully all sdio chipsets sharing
the same device id and BMI version will use the same firmware image.
Currently, these two params are the only two that are used when
selecting firmware.

struct bmi_target_info also has a type parameter, but I can't see it
being used anywhere in the code.

If it turns out that several sdio chipsets uses the same BMI version and
device id, this parameter might be used (together with the other two) to
select hw params.

Re: [RFC v2 06/11] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/htc.h |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless=
/ath/ath10k/htc.h
> index f94b25a..2963694 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.h
> +++ b/drivers/net/wireless/ath/ath10k/htc.h
> @@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
>  	ATH10K_HTC_FLAG_BUNDLE_MASK     =3D 0xF0
>  };
> =20
> +#define ATH10K_HTC_FLAG_BUNDLE_LSB         4
> +

I think you could fold patches from 6 to 11 into one patch. They are all
about adding code, and most of the commit logs are empty anyway.

--=20
Kalle Valo=

Re: [RFC v2 11/11] ath10k: Added sdio support

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> Initial HIF sdio/mailbox implementation.
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>

I know most of this coming from ath6kl but I think we should still
improve the code. Lots of comments will follow, don't get scared :)

> +#define CALC_TXRX_PADDED_LEN(ar_sdio, len) \
> +	(__ALIGN_MASK((len), (ar_sdio)->mbox_info.block_mask))

I think this could be a proper static inline function. Andrew Morton
once said: "Write in C, not CPP" (or something like that), I think
that's a really good point.

> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *=
buf,
> +				       u32 len, u32 request);
> +static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, voi=
d *buf,
> +				     size_t buf_len);
> +static int ath10k_sdio_hif_diag_read32(struct ath10k *ar, u32 address,
> +				       u32 *value);

We prefer to avoid forward declarations if at all possible. I didn't
check but if there's a clean way to avoid these please remove them.

> +/* HIF mbox interrupt handling */
> +
> +static int ath10k_sdio_mbox_rx_process_packet(struct ath10k_sdio *ar_sdi=
o,
> +					      struct ath10k_sdio_rx_data *pkt,
> +					      u32 *lookaheads,
> +					      int *n_lookaheads)
> +{

So the style in ath10k is that all functions (of course this doesn't
apply to callbacks etc) have "struct ath10k *ar" as the first parameter.
This way there's no need to check if a function takes ar or ar_sdio.

> +	int status =3D 0;

In ath10k we prefer to use ret. And avoid initialising it, please.

> +	struct ath10k_htc *htc =3D &ar_sdio->ar->htc;
> +	struct sk_buff *skb =3D pkt->skb;
> +	struct ath10k_htc_hdr *htc_hdr =3D (struct ath10k_htc_hdr *)skb->data;
> +	bool trailer_present =3D htc_hdr->flags & ATH10K_HTC_FLAG_TRAILER_PRESE=
NT;
> +	u16 payload_len;
> +
> +	payload_len =3D le16_to_cpu(htc_hdr->len);
> +
> +	if (trailer_present) {
> +		u8 *trailer;
> +		enum ath10k_htc_ep_id eid;
> +
> +		trailer =3D skb->data + sizeof(struct ath10k_htc_hdr) +
> +			  payload_len - htc_hdr->trailer_len;
> +
> +		eid =3D (enum ath10k_htc_ep_id)htc_hdr->eid;

A some kind of mapping function for ep id would be nice, it makes it
more visible how it works.

> +static int ath10k_sdio_mbox_rx_process_packets(struct ath10k_sdio *ar_sd=
io,
> +					       u32 lookaheads[],
> +					       int *n_lookahead)
> +{
> +	struct ath10k *ar =3D ar_sdio->ar;
> +	struct ath10k_htc *htc =3D &ar->htc;
> +	struct ath10k_sdio_rx_data *pkt;
> +	int status =3D 0, i;
> +
> +	for (i =3D 0; i < ar_sdio->n_rx_pkts; i++) {
> +		struct ath10k_htc_ep *ep;
> +		enum ath10k_htc_ep_id id;
> +		u32 *lookaheads_local =3D lookaheads;
> +		int *n_lookahead_local =3D n_lookahead;
> +
> +		id =3D ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
> +
> +		if (id >=3D ATH10K_HTC_EP_COUNT) {
> +			ath10k_err(ar, "Invalid endpoint in look-ahead: %d\n",
> +				   id);

In ath10k we use ath10k_err() for errors from which can't survive and
ath10k_warn() for errors where we try to continue. So ath10k_warn()
would be more approriate here and most of other cases in sdio.c

> +			status =3D -ENOMEM;
> +			goto out;
> +		}
> +
> +		ep =3D &htc->endpoint[id];
> +
> +		if (ep->service_id =3D=3D 0) {
> +			ath10k_err(ar, "ep %d is not connected !\n", id);
> +			status =3D -ENOMEM;
> +			goto out;
> +		}
> +
> +		pkt =3D &ar_sdio->rx_pkts[i];
> +
> +		if (pkt->part_of_bundle && !pkt->last_in_bundle) {
> +			/* Only read lookahead's from RX trailers
> +			 * for the last packet in a bundle.
> +			 */
> +			lookaheads_local =3D NULL;
> +			n_lookahead_local =3D NULL;
> +		}
> +
> +		status =3D ath10k_sdio_mbox_rx_process_packet(ar_sdio,
> +							    pkt,
> +							    lookaheads_local,
> +							    n_lookahead_local);
> +		if (status)
> +			goto out;
> +
> +		ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
> +		/* The RX complete handler now owns the skb...*/
> +		pkt->skb =3D NULL;
> +		pkt->alloc_len =3D 0;
> +	}
> +
> +out:
> +	/* Free all packets that was not passed on to the RX completion
> +	 * handler...
> +	 */
> +	for (; i < ar_sdio->n_rx_pkts; i++)
> +		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);

I got a bit fooled by not initialising i here and only then realised
why. I guess it's ok but a bit of so and so

> +
> +	return status;
> +}
> +
> +static int 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)
> +{
> +	int i, status =3D 0;
> +
> +	*bndl_cnt =3D (htc_hdr->flags & ATH10K_HTC_FLAG_BUNDLE_MASK) >>
> +		    ATH10K_HTC_FLAG_BUNDLE_LSB;

We recently got FIELD_GET() and FIELD_PREP() to kernel for handling
bitmasks. ath10k is not yet converted (patches welcome!) but it would be
good to use those already in sdio.c. Also SM() could be replaced with
those.

> +int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k_sdio *ar_sdio,
> +					   u32 msg_lookahead, bool *done)
> +{
> +	struct ath10k *ar =3D ar_sdio->ar;
> +	int status =3D 0;
> +	u32 lookaheads[ATH10K_SDIO_MAX_RX_MSGS];
> +	int n_lookaheads =3D 1;
> +
> +	*done =3D true;
> +
> +	/* Copy the lookahead obtained from the HTC register table into our
> +	 * temp array as a start value.
> +	 */
> +	lookaheads[0] =3D msg_lookahead;
> +
> +	for (;;) {

Iternal loops in kernel can be dangerous. Better to add some sort of
timeout check with a warning message, something like:

while ((time_before(jiffies, timeout)) {
}

if (timed out)
        ath10k_warn("timeout in foo");

> +		/* Try to allocate as many HTC RX packets indicated by
> +		 * n_lookaheads.
> +		 */
> +		status =3D ath10k_sdio_mbox_rx_alloc(ar_sdio, lookaheads,
> +						   n_lookaheads);
> +		if (status)
> +			break;
> +
> +		if (ar_sdio->n_rx_pkts >=3D 2)
> +			/* A recv bundle was detected, force IRQ status
> +			 * re-check again.
> +			 */
> +			*done =3D false;
> +
> +		status =3D ath10k_sdio_mbox_rx_fetch(ar_sdio);
> +
> +		/* Process fetched packets. This will potentially update
> +		 * n_lookaheads depending on if the packets contain lookahead
> +		 * reports.
> +		 */
> +		n_lookaheads =3D 0;
> +		status =3D ath10k_sdio_mbox_rx_process_packets(ar_sdio,
> +							     lookaheads,
> +							     &n_lookaheads);
> +
> +		if (!n_lookaheads || status)
> +			break;
> +
> +		/* For SYNCH processing, if we get here, we are running
> +		 * through the loop again due to updated lookaheads. Set
> +		 * flag that we should re-check IRQ status registers again
> +		 * before leaving IRQ processing, this can net better
> +		 * performance in high throughput situations.
> +		 */
> +		*done =3D false;
> +	}
> +
> +	if (status && (status !=3D -ECANCELED))
> +		ath10k_err(ar, "failed to get pending recv messages: %d\n",
> +			   status);
> +
> +	if (atomic_read(&ar_sdio->stopping)) {
> +		ath10k_warn(ar, "host is going to stop. Turning of RX\n");
> +		ath10k_sdio_hif_rx_control(ar_sdio, false);
> +	}

I'm always skeptic when I use atomic variables used like this, I doubt
it's really correct.

> +
> +	return status;
> +}
> +
> +static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k_sdio *ar_sdio)
> +{
> +	int ret;
> +	u32 dummy;
> +	struct ath10k *ar =3D ar_sdio->ar;
> +
> +	ath10k_warn(ar, "firmware crashed\n");

We have firmware crash dump support in ath10k. You could add a "TODO:"
comment about implementing that later.

> +static int ath10k_sdio_mbox_proc_err_intr(struct ath10k_sdio *ar_sdio)
> +{
> +	int status;
> +	u8 error_int_status;
> +	u8 reg_buf[4];
> +	struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +	struct ath10k *ar =3D ar_sdio->ar;
> +
> +	ath10k_dbg(ar, ATH10K_DBG_SDIO, "error interrupt\n");
> +
> +	error_int_status =3D irq_data->irq_proc_reg.error_int_status & 0x0F;
> +	if (!error_int_status) {
> +		WARN_ON(1);
> +		return -EIO;
> +	}
> +
> +	ath10k_dbg(ar, ATH10K_DBG_SDIO,
> +		   "valid interrupt source(s) in ERROR_INT_STATUS: 0x%x\n",
> +		   error_int_status);
> +
> +	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_WAKEUP))
> +		ath10k_dbg(ar, ATH10K_DBG_SDIO, "error : wakeup\n");
> +
> +	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_RX_UNDERFLOW))
> +		ath10k_err(ar, "rx underflow\n");
> +
> +	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_TX_OVERFLOW))
> +		ath10k_err(ar, "tx overflow\n");
> +
> +	/* Clear the interrupt */
> +	irq_data->irq_proc_reg.error_int_status &=3D ~error_int_status;
> +
> +	/* set W1C value to clear the interrupt, this hits the register first *=
/
> +	reg_buf[0] =3D error_int_status;
> +	reg_buf[1] =3D 0;
> +	reg_buf[2] =3D 0;
> +	reg_buf[3] =3D 0;
> +
> +	status =3D ath10k_sdio_read_write_sync(ar,
> +					     MBOX_ERROR_INT_STATUS_ADDRESS,
> +					     reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
> +
> +	WARN_ON(status);

This is a bit dangerous, in worst case it can spam the kernel log and
force a host reboot due watchdog timing out etc.

Better to replace with standard warning message:

ret =3D ath10k_sdio_read_write_sync(ar,
				     MBOX_ERROR_INT_STATUS_ADDRESS,
				     reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
if (ret) {
        ath10k_warn("failed to read interrupr status: %d", ret);
        return ret;
}

> +static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k_sdio *ar_sdio)
> +{
> +	int status;
> +	struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +	struct ath10k *ar =3D ar_sdio->ar;
> +	u8 cpu_int_status, reg_buf[4];
> +
> +	cpu_int_status =3D irq_data->irq_proc_reg.cpu_int_status &
> +			 irq_data->irq_en_reg.cpu_int_status_en;
> +	if (!cpu_int_status) {
> +		WARN_ON(1);
> +		return -EIO;
> +	}

Ditto about WARN_ON(), ath10k_warn() is much better.

> +/* process pending interrupts synchronously */
> +static int ath10k_sdio_mbox_proc_pending_irqs(struct ath10k_sdio *ar_sdi=
o,
> +					      bool *done)
> +{
> +	struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +	struct ath10k *ar =3D ar_sdio->ar;
> +	struct ath10k_sdio_irq_proc_registers *rg;
> +	int status =3D 0;
> +	u8 host_int_status =3D 0;
> +	u32 lookahead =3D 0;
> +	u8 htc_mbox =3D 1 << ATH10K_HTC_MAILBOX;
> +
> +	/* NOTE: HIF implementation guarantees that the context of this
> +	 * call allows us to perform SYNCHRONOUS I/O, that is we can block,
> +	 * sleep or call any API that can block or switch thread/task
> +	 * contexts. This is a fully schedulable context.
> +	 */
> +
> +	/* Process pending intr only when int_status_en is clear, it may
> +	 * result in unnecessary bus transaction otherwise. Target may be
> +	 * unresponsive at the time.
> +	 */
> +	if (irq_data->irq_en_reg.int_status_en) {
> +		/* Read the first sizeof(struct ath10k_irq_proc_registers)
> +		 * bytes of the HTC register table. This
> +		 * will yield us the value of different int status
> +		 * registers and the lookahead registers.
> +		 */
> +		status =3D ath10k_sdio_read_write_sync(
> +				ar,
> +				MBOX_HOST_INT_STATUS_ADDRESS,
> +				(u8 *)&irq_data->irq_proc_reg,
> +				sizeof(irq_data->irq_proc_reg),
> +				HIF_RD_SYNC_BYTE_INC);
> +		if (status)
> +			goto out;
> +
> +		/* Update only those registers that are enabled */
> +		host_int_status =3D irq_data->irq_proc_reg.host_int_status &
> +				  irq_data->irq_en_reg.int_status_en;
> +
> +		/* Look at mbox status */
> +		if (host_int_status & htc_mbox) {
> +			/* Mask out pending mbox value, we use look ahead as
> +			 * the real flag for mbox processing.
> +			 */
> +			host_int_status &=3D ~htc_mbox;
> +			if (irq_data->irq_proc_reg.rx_lookahead_valid &
> +			    htc_mbox) {
> +				rg =3D &irq_data->irq_proc_reg;
> +				lookahead =3D le32_to_cpu(
> +					rg->rx_lookahead[ATH10K_HTC_MAILBOX]);
> +				if (!lookahead)
> +					ath10k_err(ar, "lookahead is zero!\n");
> +			}
> +		}
> +	}
> +
> +	if (!host_int_status && !lookahead) {
> +		*done =3D true;
> +		goto out;
> +	}
> +
> +	if (lookahead) {
> +		ath10k_dbg(ar, ATH10K_DBG_SDIO,
> +			   "pending mailbox msg, lookahead: 0x%08X\n",
> +			   lookahead);
> +
> +		status =3D ath10k_sdio_mbox_rxmsg_pending_handler(ar_sdio,
> +								lookahead,
> +								done);
> +		if (status)
> +			goto out;
> +	}
> +
> +	/* now, handle the rest of the interrupts */
> +	ath10k_dbg(ar, ATH10K_DBG_SDIO,
> +		   "valid interrupt source(s) for other interrupts: 0x%x\n",
> +		   host_int_status);
> +
> +	if (MS(host_int_status, MBOX_HOST_INT_STATUS_CPU)) {
> +		/* CPU Interrupt */
> +		status =3D ath10k_sdio_mbox_proc_cpu_intr(ar_sdio);
> +		if (status)
> +			goto out;
> +	}
> +
> +	if (MS(host_int_status, MBOX_HOST_INT_STATUS_ERROR)) {
> +		/* Error Interrupt */
> +		status =3D ath10k_sdio_mbox_proc_err_intr(ar_sdio);
> +		if (status)
> +			goto out;
> +	}
> +
> +	if (MS(host_int_status, MBOX_HOST_INT_STATUS_COUNTER))
> +		/* Counter Interrupt */
> +		status =3D ath10k_sdio_mbox_proc_counter_intr(ar_sdio);
> +
> +out:
> +	/* An optimization to bypass reading the IRQ status registers
> +	 * unecessarily which can re-wake the target, if upper layers
> +	 * determine that we are in a low-throughput mode, we can rely on
> +	 * taking another interrupt rather than re-checking the status
> +	 * registers which can re-wake the target.
> +	 *
> +	 * NOTE : for host interfaces that makes use of detecting pending
> +	 * mbox messages at hif can not use this optimization due to
> +	 * possible side effects, SPI requires the host to drain all
> +	 * messages from the mailbox before exiting the ISR routine.
> +	 */
> +
> +	ath10k_dbg(ar, ATH10K_DBG_SDIO,
> +		   "%s: (done:%d, status=3D%d)\n", __func__, *done, status);

We try to follow this kind of format for debug messages:

"sdio pending irqs done %d status %d"

So start with the debug level name followed by the debug separated with spa=
ces.

And IIRC no need for "\n", the macro adds that automatically.

> +
> +	return status;
> +}
> +
> +/* 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(u8 *buf)
> +{
> +	return ((unsigned long)buf & 0x3) || !virt_addr_valid(buf);
> +}

IS_ALIGNED()? And this is super ugly, do we really need this? I would
much prefer that we would directly use struct sk_buff, more of that
later.

> +static inline enum ath10k_htc_ep_id pipe_id_to_eid(u8 pipe_id)
> +{
> +	return (enum ath10k_htc_ep_id)pipe_id;
> +}

Oh, we already have a this kind of mapping function? Can't this be used
earlier?

> +static void ath10k_sdio_set_mbox_info(struct ath10k_sdio *ar_sdio)
> +{
> +	struct ath10k_mbox_info *mbox_info =3D &ar_sdio->mbox_info;
> +	u16 device =3D ar_sdio->func->device;
> +
> +	mbox_info->htc_addr =3D ATH10K_HIF_MBOX_BASE_ADDR;
> +	mbox_info->block_size =3D ATH10K_HIF_MBOX_BLOCK_SIZE;
> +	mbox_info->block_mask =3D ATH10K_HIF_MBOX_BLOCK_SIZE - 1;
> +	mbox_info->gmbox_addr =3D ATH10K_HIF_GMBOX_BASE_ADDR;
> +	mbox_info->gmbox_sz =3D ATH10K_HIF_GMBOX_WIDTH;
> +
> +	mbox_info->ext_info[0].htc_ext_addr =3D ATH10K_HIF_MBOX0_EXT_BASE_ADDR;
> +
> +	if ((device & ATH10K_MANUFACTURER_ID_REV_MASK) < 4)
> +		mbox_info->ext_info[0].htc_ext_sz =3D ATH10K_HIF_MBOX0_EXT_WIDTH;
> +	else
> +		/* from rome 2.0(0x504), the width has been extended
> +		 * to 56K
> +		 */
> +		mbox_info->ext_info[0].htc_ext_sz =3D
> +			ATH10K_HIF_MBOX0_EXT_WIDTH_ROME_2_0;
> +
> +	mbox_info->ext_info[1].htc_ext_addr =3D
> +		mbox_info->ext_info[0].htc_ext_addr +
> +		mbox_info->ext_info[0].htc_ext_sz +
> +		ATH10K_HIF_MBOX_DUMMY_SPACE_SIZE;
> +	mbox_info->ext_info[1].htc_ext_sz =3D ATH10K_HIF_MBOX1_EXT_WIDTH;
> +}
> +
> +static inline void ath10k_sdio_set_cmd52_arg(u32 *arg, u8 write, u8 raw,
> +					     unsigned int address,
> +					     unsigned char val)
> +{
> +	const u8 func =3D 0;
> +
> +	*arg =3D ((write & 1) << 31) |
> +	       ((func & 0x7) << 28) |
> +	       ((raw & 1) << 27) |
> +	       (1 << 26) |
> +	       ((address & 0x1FFFF) << 9) |
> +	       (1 << 8) |
> +	       (val & 0xFF);
> +}

Quite ugly. FIELD_PREP() & co?

> +
> +static int ath10k_sdio_func0_cmd52_wr_byte(struct mmc_card *card,
> +					   unsigned int address,
> +					   unsigned char byte)
> +{
> +	struct mmc_command io_cmd;
> +
> +	memset(&io_cmd, 0, sizeof(io_cmd));
> +	ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 1, 0, address, byte);
> +	io_cmd.opcode =3D SD_IO_RW_DIRECT;
> +	io_cmd.flags =3D MMC_RSP_R5 | MMC_CMD_AC;
> +
> +	return mmc_wait_for_cmd(card->host, &io_cmd, 0);
> +}
> +
> +static int ath10k_sdio_func0_cmd52_rd_byte(struct mmc_card *card,
> +					   unsigned int address,
> +					   unsigned char *byte)
> +{
> +	int ret;
> +	struct mmc_command io_cmd;
> +
> +	memset(&io_cmd, 0, sizeof(io_cmd));
> +	ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 0, 0, address, 0);
> +	io_cmd.opcode =3D SD_IO_RW_DIRECT;
> +	io_cmd.flags =3D MMC_RSP_R5 | MMC_CMD_AC;
> +
> +	ret =3D mmc_wait_for_cmd(card->host, &io_cmd, 0);
> +	if (!ret)
> +		*byte =3D io_cmd.resp[0];
> +
> +	return ret;
> +}
> +
> +static int ath10k_sdio_io(struct ath10k_sdio *ar_sdio, u32 request, u32 =
addr,
> +			  u8 *buf, u32 len)
> +{
> +	int ret =3D 0;

Avoid these kind of unnecessary initialisations.

> +	struct sdio_func *func =3D ar_sdio->func;
> +	struct ath10k *ar =3D ar_sdio->ar;
> +
> +	sdio_claim_host(func);
> +
> +	if (request & HIF_WRITE) {
> +		if (request & HIF_FIXED_ADDRESS)
> +			ret =3D sdio_writesb(func, addr, buf, len);
> +		else
> +			ret =3D sdio_memcpy_toio(func, addr, buf, len);
> +	} else {
> +		if (request & HIF_FIXED_ADDRESS)
> +			ret =3D sdio_readsb(func, buf, addr, len);
> +		else
> +			ret =3D sdio_memcpy_fromio(func, buf, addr, len);
> +	}
> +
> +	sdio_release_host(func);
> +
> +	ath10k_dbg(ar, ATH10K_DBG_SDIO, "%s addr 0x%x%s buf 0x%p len %d\n",
> +		   request & HIF_WRITE ? "wr" : "rd", addr,
> +		   request & HIF_FIXED_ADDRESS ? " (fixed)" : "", buf, len);
> +	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL,
> +			request & HIF_WRITE ? "sdio wr " : "sdio rd ",
> +			buf, len);
> +
> +	return ret;
> +}
> +
> +static struct ath10k_sdio_bus_request
> +*ath10k_sdio_alloc_busreq(struct ath10k_sdio *ar_sdio)
> +{
> +	struct ath10k_sdio_bus_request *bus_req;
> +
> +	spin_lock_bh(&ar_sdio->lock);
> +
> +	if (list_empty(&ar_sdio->bus_req_freeq)) {
> +		spin_unlock_bh(&ar_sdio->lock);
> +		return NULL;
> +	}
> +
> +	bus_req =3D list_first_entry(&ar_sdio->bus_req_freeq,
> +				   struct ath10k_sdio_bus_request, list);
> +	list_del(&bus_req->list);
> +
> +	spin_unlock_bh(&ar_sdio->lock);
> +
> +	return bus_req;
> +}
> +
> +static void ath10k_sdio_free_bus_req(struct ath10k_sdio *ar_sdio,
> +				     struct ath10k_sdio_bus_request *bus_req)
> +{
> +	spin_lock_bh(&ar_sdio->lock);
> +	list_add_tail(&bus_req->list, &ar_sdio->bus_req_freeq);
> +	spin_unlock_bh(&ar_sdio->lock);
> +}
> +
> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *=
buf,
> +				       u32 len, u32 request)
> +{
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +	u8  *tbuf =3D NULL;

Extra space after u8?

> +	int ret;
> +	bool bounced =3D false;
> +
> +	if (request & HIF_BLOCK_BASIS)
> +		len =3D round_down(len, ar_sdio->mbox_info.block_size);
> +
> +	if (buf_needs_bounce(buf)) {
> +		if (!ar_sdio->dma_buffer)
> +			return -ENOMEM;
> +		/* FIXME: I am not sure if it is always correct to assume
> +		 * that the SDIO irq is a "fake" irq and sleep is possible.
> +		 * (this function will get called from
> +		 * ath10k_sdio_irq_handler
> +		 */
> +		mutex_lock(&ar_sdio->dma_buffer_mutex);
> +		tbuf =3D ar_sdio->dma_buffer;
> +
> +		if (request & HIF_WRITE)
> +			memcpy(tbuf, buf, len);
> +
> +		bounced =3D true;
> +	} else {
> +		tbuf =3D buf;
> +	}

So I really hate that ar_sdio->dma_buffer, do we really need it? What if
we just get rid of it and allocate struct sk_buff and pass that around?
No need to do extra copying then, I hope :)

> +
> +	ret =3D ath10k_sdio_io(ar_sdio, request, addr, tbuf, len);
> +	if ((request & HIF_READ) && bounced)
> +		memcpy(buf, tbuf, len);
> +
> +	if (bounced)
> +		mutex_unlock(&ar_sdio->dma_buffer_mutex);
> +
> +	return ret;
> +}
> +
> +static void __ath10k_sdio_write_async(struct ath10k_sdio *ar_sdio,
> +				      struct ath10k_sdio_bus_request *req)
> +{
> +	int status;
> +	struct ath10k_htc_ep *ep;
> +	struct sk_buff *skb;
> +
> +	skb =3D req->skb;
> +	status =3D ath10k_sdio_read_write_sync(ar_sdio->ar, req->address,
> +					     skb->data, req->len,
> +					     req->request);
> +	ep =3D &ar_sdio->ar->htc.endpoint[req->eid];
> +	ath10k_htc_notify_tx_completion(ep, skb);
> +	ath10k_sdio_free_bus_req(ar_sdio, req);
> +}
> +
> +static void ath10k_sdio_write_async_work(struct work_struct *work)
> +{
> +	struct ath10k_sdio *ar_sdio;
> +	struct ath10k_sdio_bus_request *req, *tmp_req;
> +
> +	ar_sdio =3D container_of(work, struct ath10k_sdio, wr_async_work);
> +
> +	spin_lock_bh(&ar_sdio->wr_async_lock);
> +	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
> +		list_del(&req->list);
> +		spin_unlock_bh(&ar_sdio->wr_async_lock);
> +		__ath10k_sdio_write_async(ar_sdio, req);
> +		spin_lock_bh(&ar_sdio->wr_async_lock);
> +	}
> +	spin_unlock_bh(&ar_sdio->wr_async_lock);
> +}
> +
> +static void ath10k_sdio_irq_handler(struct sdio_func *func)
> +{
> +	int status =3D 0;
> +	unsigned long timeout;
> +	struct ath10k_sdio *ar_sdio;
> +	bool done =3D false;
> +
> +	ar_sdio =3D sdio_get_drvdata(func);
> +	atomic_set(&ar_sdio->irq_handling, 1);
> +
> +	/* Release the host during interrupts so we can pick it back up when
> +	 * we process commands.
> +	 */
> +	sdio_release_host(ar_sdio->func);
> +
> +	timeout =3D jiffies + ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ;
> +	while (time_before(jiffies, timeout) && !done) {
> +		status =3D ath10k_sdio_mbox_proc_pending_irqs(ar_sdio, &done);
> +		if (status)
> +			break;
> +	}
> +
> +	sdio_claim_host(ar_sdio->func);
> +
> +	atomic_set(&ar_sdio->irq_handling, 0);
> +	wake_up(&ar_sdio->irq_wq);
> +
> +	WARN_ON(status && status !=3D -ECANCELED);
> +}

Questionable use of an atomic variable again, looks like badly implement
poor man's locking to me. And instead of wake_up() we should workqueues
or threaded irqs (if sdio supports that).

> +
> +static int ath10k_sdio_hif_disable_intrs(struct ath10k_sdio *ar_sdio)
> +{
> +	int ret;
> +	struct ath10k_sdio_irq_enable_reg regs;
> +	struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +
> +	memset(&regs, 0, sizeof(regs));
> +
> +	ret =3D ath10k_sdio_read_write_sync(ar_sdio->ar,
> +					  MBOX_INT_STATUS_ENABLE_ADDRESS,
> +					  &regs.int_status_en, sizeof(regs),
> +					  HIF_WR_SYNC_BYTE_INC);
> +	if (ret) {
> +		ath10k_err(ar_sdio->ar, "Unable to disable sdio interrupts\n");
> +		return ret;
> +	}
> +
> +	spin_lock_bh(&irq_data->lock);
> +	irq_data->irq_en_reg =3D regs;
> +	spin_unlock_bh(&irq_data->lock);
> +
> +	return 0;
> +}
> +
> +static int ath10k_sdio_hif_power_up(struct ath10k *ar)
> +{
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +	struct sdio_func *func =3D ar_sdio->func;
> +	int ret =3D 0;
> +
> +	if (!ar_sdio->is_disabled)
> +		return 0;
> +
> +	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power on\n");
> +
> +	sdio_claim_host(func);
> +
> +	ret =3D sdio_enable_func(func);
> +	if (ret) {
> +		ath10k_err(ar, "Unable to enable sdio func: %d)\n", ret);
> +		sdio_release_host(func);
> +		return ret;
> +	}
> +
> +	sdio_release_host(func);
> +
> +	/* Wait for hardware to initialise. It should take a lot less than
> +	 * 20 ms but let's be conservative here.
> +	 */
> +	msleep(20);
> +
> +	ar_sdio->is_disabled =3D false;
> +
> +	ret =3D ath10k_sdio_hif_disable_intrs(ar_sdio);
> +
> +	return ret;
> +}
> +
> +static void ath10k_sdio_hif_power_down(struct ath10k *ar)
> +{
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +	int ret;
> +
> +	if (ar_sdio->is_disabled)
> +		return;
> +
> +	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power off\n");
> +
> +	/* Disable the card */
> +	sdio_claim_host(ar_sdio->func);
> +	ret =3D sdio_disable_func(ar_sdio->func);
> +	sdio_release_host(ar_sdio->func);
> +
> +	if (ret)
> +		ath10k_dbg(ar, ATH10K_DBG_BOOT,
> +			   "Unable to disable sdio: %d\n", ret);

Shouldn't this be ath10k_warn()?

> +
> +	ar_sdio->is_disabled =3D true;
> +}
> +
> +int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
> +			  struct ath10k_hif_sg_item *items, int n_items)
> +{
> +	int i;
> +	u32 address;
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +	struct ath10k_sdio_bus_request *bus_req;
> +
> +	bus_req =3D ath10k_sdio_alloc_busreq(ar_sdio);
> +
> +	if (WARN_ON_ONCE(!bus_req))
> +		return -ENOMEM;

Is ath10k_warn() more approriate?

> +	for (i =3D 0; i < n_items; i++) {
> +		bus_req->skb =3D items[i].transfer_context;
> +		bus_req->request =3D HIF_WRITE;
> +		bus_req->eid =3D pipe_id_to_eid(pipe_id);
> +		/* Write TX data to the end of the mbox address space */
> +		address =3D ar_sdio->mbox_addr[bus_req->eid] +
> +			  ar_sdio->mbox_size[bus_req->eid] - bus_req->skb->len;
> +		bus_req->address =3D address;
> +		bus_req->len =3D CALC_TXRX_PADDED_LEN(ar_sdio, bus_req->skb->len);
> +
> +		spin_lock_bh(&ar_sdio->wr_async_lock);
> +		list_add_tail(&bus_req->list, &ar_sdio->wr_asyncq);
> +		spin_unlock_bh(&ar_sdio->wr_async_lock);
> +	}
> +
> +	queue_work(ar_sdio->workqueue, &ar_sdio->wr_async_work);
> +
> +	return 0;
> +}
> +
> +static int ath10k_sdio_hif_enable_intrs(struct ath10k_sdio *ar_sdio)
> +{
> +	struct ath10k_sdio_irq_enable_reg regs;
> +	int status;
> +	struct ath10k_sdio_irq_data *irq_data =3D &ar_sdio->irq_data;
> +
> +	memset(&regs, 0, sizeof(regs));
> +
> +	/* Enable all but CPU interrupts */
> +	regs.int_status_en =3D SM(0x01, MBOX_INT_STATUS_ENABLE_ERROR) |
> +			     SM(0x01, MBOX_INT_STATUS_ENABLE_CPU) |
> +			     SM(0x01, MBOX_INT_STATUS_ENABLE_COUNTER);
> +
> +	/* NOTE: There are some cases where HIF can do detection of
> +	 * pending mbox messages which is disabled now.
> +	 */
> +	regs.int_status_en |=3D SM(0x01, MBOX_INT_STATUS_ENABLE_MBOX_DATA);
> +
> +	/* Set up the CPU Interrupt status Register */
> +	regs.cpu_int_status_en =3D 0;
> +
> +	/* Set up the Error Interrupt status Register */
> +	regs.err_int_status_en =3D
> +		SM(0x01, MBOX_ERROR_STATUS_ENABLE_RX_UNDERFLOW) |
> +		SM(0x01, MBOX_ERROR_STATUS_ENABLE_TX_OVERFLOW);
> +
> +	/* Enable Counter interrupt status register to get fatal errors for
> +	 * debugging.
> +	 */
> +	regs.cntr_int_status_en =3D SM(ATH10K_SDIO_TARGET_DEBUG_INTR_MASK,
> +				     MBOX_COUNTER_INT_STATUS_ENABLE_BIT);
> +
> +	status =3D ath10k_sdio_read_write_sync(ar_sdio->ar,
> +					     MBOX_INT_STATUS_ENABLE_ADDRESS,
> +					     &regs.int_status_en, sizeof(regs),
> +					     HIF_WR_SYNC_BYTE_INC);
> +	if (status) {
> +		ath10k_err(ar_sdio->ar,
> +			   "failed to update interrupt ctl reg err: %d\n",
> +			   status);
> +		return status;
> +	}
> +
> +	spin_lock_bh(&irq_data->lock);
> +	irq_data->irq_en_reg =3D regs;
> +	spin_unlock_bh(&irq_data->lock);
> +
> +	return 0;
> +}
> +
> +static int ath10k_sdio_hif_start(struct ath10k *ar)
> +{
> +	int ret;
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +	u32 addr, val;
> +
> +	addr =3D host_interest_item_address(HI_ITEM(hi_acs_flags));
> +
> +	ret =3D ath10k_sdio_hif_diag_read32(ar, addr, &val);
> +	if (ret) {
> +		ath10k_err(ar, "Unable to read diag mem: %d\n", ret);
> +		goto out;
> +	}
> +
> +	if (val & HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_FW_ACK) {
> +		ath10k_dbg(ar, ATH10K_DBG_SDIO,
> +			   "Mailbox SWAP Service is enabled\n");
> +		ar_sdio->swap_mbox =3D true;
> +	}
> +
> +	/* eid 0 always uses the lower part of the extended mailbox address
> +	 * space (ext_info[0].htc_ext_addr).
> +	 */
> +	ar_sdio->mbox_addr[0] =3D ar_sdio->mbox_info.ext_info[0].htc_ext_addr;
> +	ar_sdio->mbox_size[0] =3D ar_sdio->mbox_info.ext_info[0].htc_ext_sz;
> +
> +	sdio_claim_host(ar_sdio->func);
> +
> +	/* Register the isr */
> +	ret =3D  sdio_claim_irq(ar_sdio->func, ath10k_sdio_irq_handler);
> +	if (ret) {
> +		ath10k_err(ar, "Failed to claim sdio irq: %d\n", ret);
> +		sdio_release_host(ar_sdio->func);
> +		goto out;
> +	}
> +
> +	sdio_release_host(ar_sdio->func);
> +
> +	ret =3D ath10k_sdio_hif_enable_intrs(ar_sdio);
> +	if (ret)
> +		ath10k_err(ar, "Failed to enable sdio interrupts: %d\n", ret);
> +
> +out:
> +	return ret;
> +}
> +
> +static bool ath10k_sdio_is_on_irq(struct ath10k *ar)
> +{
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +
> +	return !atomic_read(&ar_sdio->irq_handling);
> +}

Yikes.

> +
> +static void ath10k_sdio_irq_disable(struct ath10k *ar)
> +{
> +	struct ath10k_sdio *ar_sdio =3D ath10k_sdio_priv(ar);
> +	int ret;
> +
> +	sdio_claim_host(ar_sdio->func);
> +
> +	atomic_set(&ar_sdio->stopping, 1);
> +
> +	if (atomic_read(&ar_sdio->irq_handling)) {
> +		sdio_release_host(ar_sdio->func);
> +
> +		ret =3D wait_event_interruptible(ar_sdio->irq_wq,
> +					       ath10k_sdio_is_on_irq(ar));
> +		if (ret)
> +			return;
> +
> +		sdio_claim_host(ar_sdio->func);
> +	}

There has to be a better way to implement this, now it feels that it's
just reimplementing the wheel. We should have proper way to wait for
interrupt handlers and workqueues to end etc.

> +	ret =3D sdio_release_irq(ar_sdio->func);
> +	if (ret)
> +		ath10k_err(ar, "Failed to release sdio irq: %d\n", ret);
> +
> +	sdio_release_host(ar_sdio->func);
> +}
> +
> +static int ath10k_sdio_config(struct ath10k_sdio *ar_sdio)
> +{
> +	struct ath10k *ar =3D ar_sdio->ar;
> +	struct sdio_func *func =3D ar_sdio->func;
> +	unsigned char byte, asyncintdelay =3D 2;
> +	int ret;
> +
> +	ath10k_dbg(ar, ATH10K_DBG_BOOT, "SDIO configuration\n");
> +
> +	sdio_claim_host(func);
> +
> +	byte =3D 0;
> +	ret =3D ath10k_sdio_func0_cmd52_rd_byte(func->card,
> +					      SDIO_CCCR_DRIVE_STRENGTH,
> +					      &byte);
> +
> +	byte =3D (byte & (~(SDIO_DRIVE_DTSx_MASK << SDIO_DRIVE_DTSx_SHIFT))) |
> +		SDIO_DTSx_SET_TYPE_D;

FIELD_PREP(). There are lots of places where that an be used.

> +static void ath10k_sdio_write32(struct ath10k *ar, u32 offset, u32 value=
)
> +{
> +}
> +
> +static u32 ath10k_sdio_read32(struct ath10k *ar, u32 offset)
> +{
> +	return 0;
> +}

Somekind of FIXME/TODO comments for write32() and read32()? What
functionality are we going to miss when we don't implement these?

> +static void ath10k_sdio_hif_send_complete_check(struct ath10k *ar,
> +						u8 pipe, int force)
> +{
> +}
> +
> +static u16 ath10k_sdio_hif_get_free_queue_number(struct ath10k *ar, u8 p=
ipe)
> +{
> +	return 0;
> +}

Similar comments here also.

> +
> +static const struct ath10k_hif_ops ath10k_sdio_hif_ops =3D {
> +	.tx_sg			=3D ath10k_sdio_hif_tx_sg,
> +	.diag_read		=3D ath10k_sdio_hif_diag_read,
> +	.diag_write		=3D ath10k_sdio_diag_write_mem,
> +	.exchange_bmi_msg	=3D ath10k_sdio_hif_exchange_bmi_msg,
> +	.start			=3D ath10k_sdio_hif_start,
> +	.stop			=3D ath10k_sdio_hif_stop,
> +	.map_service_to_pipe	=3D ath10k_sdio_hif_map_service_to_pipe,
> +	.get_default_pipe	=3D ath10k_sdio_hif_get_default_pipe,
> +	.send_complete_check	=3D ath10k_sdio_hif_send_complete_check,
> +	.get_free_queue_number	=3D ath10k_sdio_hif_get_free_queue_number,
> +	.power_up		=3D ath10k_sdio_hif_power_up,
> +	.power_down		=3D ath10k_sdio_hif_power_down,
> +	.read32			=3D ath10k_sdio_read32,
> +	.write32		=3D ath10k_sdio_write32,
> +#ifdef CONFIG_PM
> +	.suspend		=3D ath10k_sdio_hif_suspend,
> +	.resume			=3D ath10k_sdio_hif_resume,
> +#endif
> +	.fetch_cal_eeprom	=3D ath10k_sdio_hif_fetch_cal_eeprom,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +
> +/* Empty handlers so that mmc subsystem doesn't remove us entirely durin=
g
> + * suspend. We instead follow cfg80211 suspend/resume handlers.
> + */
> +static int ath10k_sdio_pm_suspend(struct device *device)
> +{
> +	return 0;
> +}
> +
> +static int ath10k_sdio_pm_resume(struct device *device)
> +{
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(ath10k_sdio_pm_ops, ath10k_sdio_pm_suspend,
> +			 ath10k_sdio_pm_resume);
> +
> +#define ATH10K_SDIO_PM_OPS (&ath10k_sdio_pm_ops)
> +
> +#else
> +
> +#define ATH10K_SDIO_PM_OPS NULL
> +
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static int ath10k_sdio_probe(struct sdio_func *func,
> +			     const struct sdio_device_id *id)
> +{
> +	int ret;
> +	struct ath10k_sdio *ar_sdio;
> +	struct ath10k *ar;
> +	int i;
> +	enum ath10k_hw_rev hw_rev;
> +
> +	hw_rev =3D ATH10K_HW_QCA6174;

This needs a comment, at least to remind if ever add something else than
QCA6174 based SDIO design.

> +
> +	ar =3D ath10k_core_create(sizeof(*ar_sdio), &func->dev, ATH10K_BUS_SDIO=
,
> +				hw_rev, &ath10k_sdio_hif_ops);
> +	if (!ar) {
> +		dev_err(&func->dev, "failed to allocate core\n");
> +		return -ENOMEM;
> +	}
> +
> +	ath10k_dbg(ar, ATH10K_DBG_BOOT,
> +		   "sdio new func %d vendor 0x%x device 0x%x block 0x%x/0x%x\n",
> +		   func->num, func->vendor, func->device,
> +		   func->max_blksize, func->cur_blksize);
> +
> +	ar_sdio =3D ath10k_sdio_priv(ar);
> +
> +	ar_sdio->dma_buffer =3D kzalloc(ATH10K_HIF_DMA_BUFFER_SIZE, GFP_KERNEL)=
;
> +	if (!ar_sdio->dma_buffer) {
> +		ret =3D -ENOMEM;
> +		goto err_core_destroy;
> +	}
> +
> +	ar_sdio->func =3D func;
> +	sdio_set_drvdata(func, ar_sdio);
> +
> +	ar_sdio->is_disabled =3D true;
> +	ar_sdio->ar =3D ar;
> +
> +	spin_lock_init(&ar_sdio->lock);
> +	spin_lock_init(&ar_sdio->wr_async_lock);
> +	spin_lock_init(&ar_sdio->irq_data.lock);
> +	mutex_init(&ar_sdio->dma_buffer_mutex);
> +
> +	INIT_LIST_HEAD(&ar_sdio->bus_req_freeq);
> +	INIT_LIST_HEAD(&ar_sdio->wr_asyncq);
> +
> +	INIT_WORK(&ar_sdio->wr_async_work, ath10k_sdio_write_async_work);
> +	ar_sdio->workqueue =3D create_singlethread_workqueue("ath10k_sdio_wq");
> +	if (!ar_sdio->workqueue)
> +		goto err;
> +
> +	init_waitqueue_head(&ar_sdio->irq_wq);
> +
> +	for (i =3D 0; i < ATH10K_SDIO_BUS_REQUEST_MAX_NUM; i++)
> +		ath10k_sdio_free_bus_req(ar_sdio, &ar_sdio->bus_req[i]);
> +
> +	ar->dev_id =3D id->device;
> +	ar->id.vendor =3D id->vendor;
> +	ar->id.device =3D id->device;
> +
> +	ath10k_sdio_set_mbox_info(ar_sdio);
> +
> +	ret =3D ath10k_sdio_config(ar_sdio);
> +	if (ret) {
> +		ath10k_err(ar, "Failed to config sdio: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret =3D ath10k_core_register(ar, 0/*chip_id is not applicaple for SDIO*=
/);
> +	if (ret) {
> +		ath10k_err(ar, "failed to register driver core: %d\n", ret);
> +		goto err;
> +	}

I would assume that chip_id is applicaple also with SDIO, there has to
be a register where to get it. Also this kind of comment style is
preferred:

/* TODO: don't know yet how to get chip_id with SDIO */
chip_id =3D 0;

ret =3D ath10k_core_register(ar, chip_id);

> +
> +	return ret;
> +
> +err:
> +	kfree(ar_sdio->dma_buffer);
> +err_core_destroy:
> +	ath10k_core_destroy(ar);
> +
> +	return ret;
> +}
> +
> +static void ath10k_sdio_remove(struct sdio_func *func)
> +{
> +	struct ath10k_sdio *ar_sdio;
> +	struct ath10k *ar;
> +
> +	ar_sdio =3D sdio_get_drvdata(func);
> +	ar =3D ar_sdio->ar;
> +
> +	ath10k_dbg(ar, ATH10K_DBG_BOOT,
> +		   "sdio removed func %d vendor 0x%x device 0x%x\n",
> +		   func->num, func->vendor, func->device);
> +
> +	(void)ath10k_sdio_hif_disable_intrs(ar_sdio);
> +	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[] =3D {
> +	{SDIO_DEVICE(ATH10K_MANUFACTURER_CODE,
> +		     (ATH10K_MANUFACTURER_ID_AR6005_BASE | 0xA))},
> +	{},
> +};

I suspect there's a more sensible way to create the device table than
this, just no time to check it now. Anyone know?

The naming "ath10k manufacturer" is also wrong, it should be QCA or
Qualcomm.

--=20
Kalle Valo=

Re: [RFC v2 11/11] ath10k: Added sdio support

[Erik Stromdahl]

On 12/16/2016 12:21 PM, Valo, Kalle wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> 
>> Initial HIF sdio/mailbox implementation.
>>
>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
> 
> I know most of this coming from ath6kl but I think we should still
> improve the code. Lots of comments will follow, don't get scared :)
> 

I have started to fix most of the review comments below and added a new
branch on my git repo (https://github.com/erstrom/linux-ath) for this:

ath-201612150919-ath10k-sdio-kvalo-review

The changes are quite massive and I am not entirely finished yet.
I will of course squash these commits before submitting a new RFC.
You can have a look to see where I am heading.

The dma_buffer removal was a little bit tricky since there are a lot of
places in the code where the sdio functions are called with stack
allocated buffers. This does not seem to be a problem on the hardware I
am running (NXP/Freescale iMX6ul) but I am afraid that there could be
problems on other architectures.

Therefore I have introduced some temporary dynamically allocated buffers
on a few places.

>> +#define CALC_TXRX_PADDED_LEN(ar_sdio, len) \
>> +	(__ALIGN_MASK((len), (ar_sdio)->mbox_info.block_mask))
> 
> I think this could be a proper static inline function. Andrew Morton
> once said: "Write in C, not CPP" (or something like that), I think
> that's a really good point.
> 
>> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *buf,
>> +				       u32 len, u32 request);
>> +static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
>> +				     size_t buf_len);
>> +static int ath10k_sdio_hif_diag_read32(struct ath10k *ar, u32 address,
>> +				       u32 *value);
> 
> We prefer to avoid forward declarations if at all possible. I didn't
> check but if there's a clean way to avoid these please remove them.
> 
>> +/* HIF mbox interrupt handling */
>> +
>> +static int ath10k_sdio_mbox_rx_process_packet(struct ath10k_sdio *ar_sdio,
>> +					      struct ath10k_sdio_rx_data *pkt,
>> +					      u32 *lookaheads,
>> +					      int *n_lookaheads)
>> +{
> 
> So the style in ath10k is that all functions (of course this doesn't
> apply to callbacks etc) have "struct ath10k *ar" as the first parameter.
> This way there's no need to check if a function takes ar or ar_sdio.
> 
>> +	int status = 0;
> 
> In ath10k we prefer to use ret. And avoid initialising it, please.
> 
>> +	struct ath10k_htc *htc = &ar_sdio->ar->htc;
>> +	struct sk_buff *skb = pkt->skb;
>> +	struct ath10k_htc_hdr *htc_hdr = (struct ath10k_htc_hdr *)skb->data;
>> +	bool trailer_present = htc_hdr->flags & ATH10K_HTC_FLAG_TRAILER_PRESENT;
>> +	u16 payload_len;
>> +
>> +	payload_len = le16_to_cpu(htc_hdr->len);
>> +
>> +	if (trailer_present) {
>> +		u8 *trailer;
>> +		enum ath10k_htc_ep_id eid;
>> +
>> +		trailer = skb->data + sizeof(struct ath10k_htc_hdr) +
>> +			  payload_len - htc_hdr->trailer_len;
>> +
>> +		eid = (enum ath10k_htc_ep_id)htc_hdr->eid;
> 
> A some kind of mapping function for ep id would be nice, it makes it
> more visible how it works.
> 
>> +static int ath10k_sdio_mbox_rx_process_packets(struct ath10k_sdio *ar_sdio,
>> +					       u32 lookaheads[],
>> +					       int *n_lookahead)
>> +{
>> +	struct ath10k *ar = ar_sdio->ar;
>> +	struct ath10k_htc *htc = &ar->htc;
>> +	struct ath10k_sdio_rx_data *pkt;
>> +	int status = 0, i;
>> +
>> +	for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
>> +		struct ath10k_htc_ep *ep;
>> +		enum ath10k_htc_ep_id id;
>> +		u32 *lookaheads_local = lookaheads;
>> +		int *n_lookahead_local = n_lookahead;
>> +
>> +		id = ((struct ath10k_htc_hdr *)&lookaheads[i])->eid;
>> +
>> +		if (id >= ATH10K_HTC_EP_COUNT) {
>> +			ath10k_err(ar, "Invalid endpoint in look-ahead: %d\n",
>> +				   id);
> 
> In ath10k we use ath10k_err() for errors from which can't survive and
> ath10k_warn() for errors where we try to continue. So ath10k_warn()
> would be more approriate here and most of other cases in sdio.c
> 
>> +			status = -ENOMEM;
>> +			goto out;
>> +		}
>> +
>> +		ep = &htc->endpoint[id];
>> +
>> +		if (ep->service_id == 0) {
>> +			ath10k_err(ar, "ep %d is not connected !\n", id);
>> +			status = -ENOMEM;
>> +			goto out;
>> +		}
>> +
>> +		pkt = &ar_sdio->rx_pkts[i];
>> +
>> +		if (pkt->part_of_bundle && !pkt->last_in_bundle) {
>> +			/* Only read lookahead's from RX trailers
>> +			 * for the last packet in a bundle.
>> +			 */
>> +			lookaheads_local = NULL;
>> +			n_lookahead_local = NULL;
>> +		}
>> +
>> +		status = ath10k_sdio_mbox_rx_process_packet(ar_sdio,
>> +							    pkt,
>> +							    lookaheads_local,
>> +							    n_lookahead_local);
>> +		if (status)
>> +			goto out;
>> +
>> +		ep->ep_ops.ep_rx_complete(ar_sdio->ar, pkt->skb);
>> +		/* The RX complete handler now owns the skb...*/
>> +		pkt->skb = NULL;
>> +		pkt->alloc_len = 0;
>> +	}
>> +
>> +out:
>> +	/* Free all packets that was not passed on to the RX completion
>> +	 * handler...
>> +	 */
>> +	for (; i < ar_sdio->n_rx_pkts; i++)
>> +		ath10k_sdio_mbox_free_rx_pkt(&ar_sdio->rx_pkts[i]);
> 
> I got a bit fooled by not initialising i here and only then realised
> why. I guess it's ok but a bit of so and so
> 
>> +
>> +	return status;
>> +}
>> +
>> +static int 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)
>> +{
>> +	int i, status = 0;
>> +
>> +	*bndl_cnt = (htc_hdr->flags & ATH10K_HTC_FLAG_BUNDLE_MASK) >>
>> +		    ATH10K_HTC_FLAG_BUNDLE_LSB;
> 
> We recently got FIELD_GET() and FIELD_PREP() to kernel for handling
> bitmasks. ath10k is not yet converted (patches welcome!) but it would be
> good to use those already in sdio.c. Also SM() could be replaced with
> those.
> 
>> +int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k_sdio *ar_sdio,
>> +					   u32 msg_lookahead, bool *done)
>> +{
>> +	struct ath10k *ar = ar_sdio->ar;
>> +	int status = 0;
>> +	u32 lookaheads[ATH10K_SDIO_MAX_RX_MSGS];
>> +	int n_lookaheads = 1;
>> +
>> +	*done = true;
>> +
>> +	/* Copy the lookahead obtained from the HTC register table into our
>> +	 * temp array as a start value.
>> +	 */
>> +	lookaheads[0] = msg_lookahead;
>> +
>> +	for (;;) {
> 
> Iternal loops in kernel can be dangerous. Better to add some sort of
> timeout check with a warning message, something like:
> 
> while ((time_before(jiffies, timeout)) {
> }
> 
> if (timed out)
>         ath10k_warn("timeout in foo");
> 
>> +		/* Try to allocate as many HTC RX packets indicated by
>> +		 * n_lookaheads.
>> +		 */
>> +		status = ath10k_sdio_mbox_rx_alloc(ar_sdio, lookaheads,
>> +						   n_lookaheads);
>> +		if (status)
>> +			break;
>> +
>> +		if (ar_sdio->n_rx_pkts >= 2)
>> +			/* A recv bundle was detected, force IRQ status
>> +			 * re-check again.
>> +			 */
>> +			*done = false;
>> +
>> +		status = ath10k_sdio_mbox_rx_fetch(ar_sdio);
>> +
>> +		/* Process fetched packets. This will potentially update
>> +		 * n_lookaheads depending on if the packets contain lookahead
>> +		 * reports.
>> +		 */
>> +		n_lookaheads = 0;
>> +		status = ath10k_sdio_mbox_rx_process_packets(ar_sdio,
>> +							     lookaheads,
>> +							     &n_lookaheads);
>> +
>> +		if (!n_lookaheads || status)
>> +			break;
>> +
>> +		/* For SYNCH processing, if we get here, we are running
>> +		 * through the loop again due to updated lookaheads. Set
>> +		 * flag that we should re-check IRQ status registers again
>> +		 * before leaving IRQ processing, this can net better
>> +		 * performance in high throughput situations.
>> +		 */
>> +		*done = false;
>> +	}
>> +
>> +	if (status && (status != -ECANCELED))
>> +		ath10k_err(ar, "failed to get pending recv messages: %d\n",
>> +			   status);
>> +
>> +	if (atomic_read(&ar_sdio->stopping)) {
>> +		ath10k_warn(ar, "host is going to stop. Turning of RX\n");
>> +		ath10k_sdio_hif_rx_control(ar_sdio, false);
>> +	}
> 
> I'm always skeptic when I use atomic variables used like this, I doubt
> it's really correct.
> 
>> +
>> +	return status;
>> +}
>> +
>> +static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k_sdio *ar_sdio)
>> +{
>> +	int ret;
>> +	u32 dummy;
>> +	struct ath10k *ar = ar_sdio->ar;
>> +
>> +	ath10k_warn(ar, "firmware crashed\n");
> 
> We have firmware crash dump support in ath10k. You could add a "TODO:"
> comment about implementing that later.
> 
>> +static int ath10k_sdio_mbox_proc_err_intr(struct ath10k_sdio *ar_sdio)
>> +{
>> +	int status;
>> +	u8 error_int_status;
>> +	u8 reg_buf[4];
>> +	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
>> +	struct ath10k *ar = ar_sdio->ar;
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_SDIO, "error interrupt\n");
>> +
>> +	error_int_status = irq_data->irq_proc_reg.error_int_status & 0x0F;
>> +	if (!error_int_status) {
>> +		WARN_ON(1);
>> +		return -EIO;
>> +	}
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_SDIO,
>> +		   "valid interrupt source(s) in ERROR_INT_STATUS: 0x%x\n",
>> +		   error_int_status);
>> +
>> +	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_WAKEUP))
>> +		ath10k_dbg(ar, ATH10K_DBG_SDIO, "error : wakeup\n");
>> +
>> +	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_RX_UNDERFLOW))
>> +		ath10k_err(ar, "rx underflow\n");
>> +
>> +	if (MS(error_int_status, MBOX_ERROR_INT_STATUS_TX_OVERFLOW))
>> +		ath10k_err(ar, "tx overflow\n");
>> +
>> +	/* Clear the interrupt */
>> +	irq_data->irq_proc_reg.error_int_status &= ~error_int_status;
>> +
>> +	/* set W1C value to clear the interrupt, this hits the register first */
>> +	reg_buf[0] = error_int_status;
>> +	reg_buf[1] = 0;
>> +	reg_buf[2] = 0;
>> +	reg_buf[3] = 0;
>> +
>> +	status = ath10k_sdio_read_write_sync(ar,
>> +					     MBOX_ERROR_INT_STATUS_ADDRESS,
>> +					     reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
>> +
>> +	WARN_ON(status);
> 
> This is a bit dangerous, in worst case it can spam the kernel log and
> force a host reboot due watchdog timing out etc.
> 
> Better to replace with standard warning message:
> 
> ret = ath10k_sdio_read_write_sync(ar,
> 				     MBOX_ERROR_INT_STATUS_ADDRESS,
> 				     reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
> if (ret) {
>         ath10k_warn("failed to read interrupr status: %d", ret);
>         return ret;
> }
> 
>> +static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k_sdio *ar_sdio)
>> +{
>> +	int status;
>> +	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
>> +	struct ath10k *ar = ar_sdio->ar;
>> +	u8 cpu_int_status, reg_buf[4];
>> +
>> +	cpu_int_status = irq_data->irq_proc_reg.cpu_int_status &
>> +			 irq_data->irq_en_reg.cpu_int_status_en;
>> +	if (!cpu_int_status) {
>> +		WARN_ON(1);
>> +		return -EIO;
>> +	}
> 
> Ditto about WARN_ON(), ath10k_warn() is much better.
> 
>> +/* process pending interrupts synchronously */
>> +static int ath10k_sdio_mbox_proc_pending_irqs(struct ath10k_sdio *ar_sdio,
>> +					      bool *done)
>> +{
>> +	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
>> +	struct ath10k *ar = ar_sdio->ar;
>> +	struct ath10k_sdio_irq_proc_registers *rg;
>> +	int status = 0;
>> +	u8 host_int_status = 0;
>> +	u32 lookahead = 0;
>> +	u8 htc_mbox = 1 << ATH10K_HTC_MAILBOX;
>> +
>> +	/* NOTE: HIF implementation guarantees that the context of this
>> +	 * call allows us to perform SYNCHRONOUS I/O, that is we can block,
>> +	 * sleep or call any API that can block or switch thread/task
>> +	 * contexts. This is a fully schedulable context.
>> +	 */
>> +
>> +	/* Process pending intr only when int_status_en is clear, it may
>> +	 * result in unnecessary bus transaction otherwise. Target may be
>> +	 * unresponsive at the time.
>> +	 */
>> +	if (irq_data->irq_en_reg.int_status_en) {
>> +		/* Read the first sizeof(struct ath10k_irq_proc_registers)
>> +		 * bytes of the HTC register table. This
>> +		 * will yield us the value of different int status
>> +		 * registers and the lookahead registers.
>> +		 */
>> +		status = ath10k_sdio_read_write_sync(
>> +				ar,
>> +				MBOX_HOST_INT_STATUS_ADDRESS,
>> +				(u8 *)&irq_data->irq_proc_reg,
>> +				sizeof(irq_data->irq_proc_reg),
>> +				HIF_RD_SYNC_BYTE_INC);
>> +		if (status)
>> +			goto out;
>> +
>> +		/* Update only those registers that are enabled */
>> +		host_int_status = irq_data->irq_proc_reg.host_int_status &
>> +				  irq_data->irq_en_reg.int_status_en;
>> +
>> +		/* Look at mbox status */
>> +		if (host_int_status & htc_mbox) {
>> +			/* Mask out pending mbox value, we use look ahead as
>> +			 * the real flag for mbox processing.
>> +			 */
>> +			host_int_status &= ~htc_mbox;
>> +			if (irq_data->irq_proc_reg.rx_lookahead_valid &
>> +			    htc_mbox) {
>> +				rg = &irq_data->irq_proc_reg;
>> +				lookahead = le32_to_cpu(
>> +					rg->rx_lookahead[ATH10K_HTC_MAILBOX]);
>> +				if (!lookahead)
>> +					ath10k_err(ar, "lookahead is zero!\n");
>> +			}
>> +		}
>> +	}
>> +
>> +	if (!host_int_status && !lookahead) {
>> +		*done = true;
>> +		goto out;
>> +	}
>> +
>> +	if (lookahead) {
>> +		ath10k_dbg(ar, ATH10K_DBG_SDIO,
>> +			   "pending mailbox msg, lookahead: 0x%08X\n",
>> +			   lookahead);
>> +
>> +		status = ath10k_sdio_mbox_rxmsg_pending_handler(ar_sdio,
>> +								lookahead,
>> +								done);
>> +		if (status)
>> +			goto out;
>> +	}
>> +
>> +	/* now, handle the rest of the interrupts */
>> +	ath10k_dbg(ar, ATH10K_DBG_SDIO,
>> +		   "valid interrupt source(s) for other interrupts: 0x%x\n",
>> +		   host_int_status);
>> +
>> +	if (MS(host_int_status, MBOX_HOST_INT_STATUS_CPU)) {
>> +		/* CPU Interrupt */
>> +		status = ath10k_sdio_mbox_proc_cpu_intr(ar_sdio);
>> +		if (status)
>> +			goto out;
>> +	}
>> +
>> +	if (MS(host_int_status, MBOX_HOST_INT_STATUS_ERROR)) {
>> +		/* Error Interrupt */
>> +		status = ath10k_sdio_mbox_proc_err_intr(ar_sdio);
>> +		if (status)
>> +			goto out;
>> +	}
>> +
>> +	if (MS(host_int_status, MBOX_HOST_INT_STATUS_COUNTER))
>> +		/* Counter Interrupt */
>> +		status = ath10k_sdio_mbox_proc_counter_intr(ar_sdio);
>> +
>> +out:
>> +	/* An optimization to bypass reading the IRQ status registers
>> +	 * unecessarily which can re-wake the target, if upper layers
>> +	 * determine that we are in a low-throughput mode, we can rely on
>> +	 * taking another interrupt rather than re-checking the status
>> +	 * registers which can re-wake the target.
>> +	 *
>> +	 * NOTE : for host interfaces that makes use of detecting pending
>> +	 * mbox messages at hif can not use this optimization due to
>> +	 * possible side effects, SPI requires the host to drain all
>> +	 * messages from the mailbox before exiting the ISR routine.
>> +	 */
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_SDIO,
>> +		   "%s: (done:%d, status=%d)\n", __func__, *done, status);
> 
> We try to follow this kind of format for debug messages:
> 
> "sdio pending irqs done %d status %d"
> 
> So start with the debug level name followed by the debug separated with spaces.
> 
> And IIRC no need for "\n", the macro adds that automatically.
> 
>> +
>> +	return status;
>> +}
>> +
>> +/* 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(u8 *buf)
>> +{
>> +	return ((unsigned long)buf & 0x3) || !virt_addr_valid(buf);
>> +}
> 
> IS_ALIGNED()? And this is super ugly, do we really need this? I would
> much prefer that we would directly use struct sk_buff, more of that
> later.
> 
>> +static inline enum ath10k_htc_ep_id pipe_id_to_eid(u8 pipe_id)
>> +{
>> +	return (enum ath10k_htc_ep_id)pipe_id;
>> +}
> 
> Oh, we already have a this kind of mapping function? Can't this be used
> earlier?
> 
>> +static void ath10k_sdio_set_mbox_info(struct ath10k_sdio *ar_sdio)
>> +{
>> +	struct ath10k_mbox_info *mbox_info = &ar_sdio->mbox_info;
>> +	u16 device = ar_sdio->func->device;
>> +
>> +	mbox_info->htc_addr = ATH10K_HIF_MBOX_BASE_ADDR;
>> +	mbox_info->block_size = ATH10K_HIF_MBOX_BLOCK_SIZE;
>> +	mbox_info->block_mask = ATH10K_HIF_MBOX_BLOCK_SIZE - 1;
>> +	mbox_info->gmbox_addr = ATH10K_HIF_GMBOX_BASE_ADDR;
>> +	mbox_info->gmbox_sz = ATH10K_HIF_GMBOX_WIDTH;
>> +
>> +	mbox_info->ext_info[0].htc_ext_addr = ATH10K_HIF_MBOX0_EXT_BASE_ADDR;
>> +
>> +	if ((device & ATH10K_MANUFACTURER_ID_REV_MASK) < 4)
>> +		mbox_info->ext_info[0].htc_ext_sz = ATH10K_HIF_MBOX0_EXT_WIDTH;
>> +	else
>> +		/* from rome 2.0(0x504), the width has been extended
>> +		 * to 56K
>> +		 */
>> +		mbox_info->ext_info[0].htc_ext_sz =
>> +			ATH10K_HIF_MBOX0_EXT_WIDTH_ROME_2_0;
>> +
>> +	mbox_info->ext_info[1].htc_ext_addr =
>> +		mbox_info->ext_info[0].htc_ext_addr +
>> +		mbox_info->ext_info[0].htc_ext_sz +
>> +		ATH10K_HIF_MBOX_DUMMY_SPACE_SIZE;
>> +	mbox_info->ext_info[1].htc_ext_sz = ATH10K_HIF_MBOX1_EXT_WIDTH;
>> +}
>> +
>> +static inline void ath10k_sdio_set_cmd52_arg(u32 *arg, u8 write, u8 raw,
>> +					     unsigned int address,
>> +					     unsigned char val)
>> +{
>> +	const u8 func = 0;
>> +
>> +	*arg = ((write & 1) << 31) |
>> +	       ((func & 0x7) << 28) |
>> +	       ((raw & 1) << 27) |
>> +	       (1 << 26) |
>> +	       ((address & 0x1FFFF) << 9) |
>> +	       (1 << 8) |
>> +	       (val & 0xFF);
>> +}
> 
> Quite ugly. FIELD_PREP() & co?
> 
>> +
>> +static int ath10k_sdio_func0_cmd52_wr_byte(struct mmc_card *card,
>> +					   unsigned int address,
>> +					   unsigned char byte)
>> +{
>> +	struct mmc_command io_cmd;
>> +
>> +	memset(&io_cmd, 0, sizeof(io_cmd));
>> +	ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 1, 0, address, byte);
>> +	io_cmd.opcode = SD_IO_RW_DIRECT;
>> +	io_cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>> +
>> +	return mmc_wait_for_cmd(card->host, &io_cmd, 0);
>> +}
>> +
>> +static int ath10k_sdio_func0_cmd52_rd_byte(struct mmc_card *card,
>> +					   unsigned int address,
>> +					   unsigned char *byte)
>> +{
>> +	int ret;
>> +	struct mmc_command io_cmd;
>> +
>> +	memset(&io_cmd, 0, sizeof(io_cmd));
>> +	ath10k_sdio_set_cmd52_arg(&io_cmd.arg, 0, 0, address, 0);
>> +	io_cmd.opcode = SD_IO_RW_DIRECT;
>> +	io_cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>> +
>> +	ret = mmc_wait_for_cmd(card->host, &io_cmd, 0);
>> +	if (!ret)
>> +		*byte = io_cmd.resp[0];
>> +
>> +	return ret;
>> +}
>> +
>> +static int ath10k_sdio_io(struct ath10k_sdio *ar_sdio, u32 request, u32 addr,
>> +			  u8 *buf, u32 len)
>> +{
>> +	int ret = 0;
> 
> Avoid these kind of unnecessary initialisations.
> 
>> +	struct sdio_func *func = ar_sdio->func;
>> +	struct ath10k *ar = ar_sdio->ar;
>> +
>> +	sdio_claim_host(func);
>> +
>> +	if (request & HIF_WRITE) {
>> +		if (request & HIF_FIXED_ADDRESS)
>> +			ret = sdio_writesb(func, addr, buf, len);
>> +		else
>> +			ret = sdio_memcpy_toio(func, addr, buf, len);
>> +	} else {
>> +		if (request & HIF_FIXED_ADDRESS)
>> +			ret = sdio_readsb(func, buf, addr, len);
>> +		else
>> +			ret = sdio_memcpy_fromio(func, buf, addr, len);
>> +	}
>> +
>> +	sdio_release_host(func);
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_SDIO, "%s addr 0x%x%s buf 0x%p len %d\n",
>> +		   request & HIF_WRITE ? "wr" : "rd", addr,
>> +		   request & HIF_FIXED_ADDRESS ? " (fixed)" : "", buf, len);
>> +	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL,
>> +			request & HIF_WRITE ? "sdio wr " : "sdio rd ",
>> +			buf, len);
>> +
>> +	return ret;
>> +}
>> +
>> +static struct ath10k_sdio_bus_request
>> +*ath10k_sdio_alloc_busreq(struct ath10k_sdio *ar_sdio)
>> +{
>> +	struct ath10k_sdio_bus_request *bus_req;
>> +
>> +	spin_lock_bh(&ar_sdio->lock);
>> +
>> +	if (list_empty(&ar_sdio->bus_req_freeq)) {
>> +		spin_unlock_bh(&ar_sdio->lock);
>> +		return NULL;
>> +	}
>> +
>> +	bus_req = list_first_entry(&ar_sdio->bus_req_freeq,
>> +				   struct ath10k_sdio_bus_request, list);
>> +	list_del(&bus_req->list);
>> +
>> +	spin_unlock_bh(&ar_sdio->lock);
>> +
>> +	return bus_req;
>> +}
>> +
>> +static void ath10k_sdio_free_bus_req(struct ath10k_sdio *ar_sdio,
>> +				     struct ath10k_sdio_bus_request *bus_req)
>> +{
>> +	spin_lock_bh(&ar_sdio->lock);
>> +	list_add_tail(&bus_req->list, &ar_sdio->bus_req_freeq);
>> +	spin_unlock_bh(&ar_sdio->lock);
>> +}
>> +
>> +static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *buf,
>> +				       u32 len, u32 request)
>> +{
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +	u8  *tbuf = NULL;
> 
> Extra space after u8?
> 
>> +	int ret;
>> +	bool bounced = false;
>> +
>> +	if (request & HIF_BLOCK_BASIS)
>> +		len = round_down(len, ar_sdio->mbox_info.block_size);
>> +
>> +	if (buf_needs_bounce(buf)) {
>> +		if (!ar_sdio->dma_buffer)
>> +			return -ENOMEM;
>> +		/* FIXME: I am not sure if it is always correct to assume
>> +		 * that the SDIO irq is a "fake" irq and sleep is possible.
>> +		 * (this function will get called from
>> +		 * ath10k_sdio_irq_handler
>> +		 */
>> +		mutex_lock(&ar_sdio->dma_buffer_mutex);
>> +		tbuf = ar_sdio->dma_buffer;
>> +
>> +		if (request & HIF_WRITE)
>> +			memcpy(tbuf, buf, len);
>> +
>> +		bounced = true;
>> +	} else {
>> +		tbuf = buf;
>> +	}
> 
> So I really hate that ar_sdio->dma_buffer, do we really need it? What if
> we just get rid of it and allocate struct sk_buff and pass that around?
> No need to do extra copying then, I hope :)
> 
>> +
>> +	ret = ath10k_sdio_io(ar_sdio, request, addr, tbuf, len);
>> +	if ((request & HIF_READ) && bounced)
>> +		memcpy(buf, tbuf, len);
>> +
>> +	if (bounced)
>> +		mutex_unlock(&ar_sdio->dma_buffer_mutex);
>> +
>> +	return ret;
>> +}
>> +
>> +static void __ath10k_sdio_write_async(struct ath10k_sdio *ar_sdio,
>> +				      struct ath10k_sdio_bus_request *req)
>> +{
>> +	int status;
>> +	struct ath10k_htc_ep *ep;
>> +	struct sk_buff *skb;
>> +
>> +	skb = req->skb;
>> +	status = ath10k_sdio_read_write_sync(ar_sdio->ar, req->address,
>> +					     skb->data, req->len,
>> +					     req->request);
>> +	ep = &ar_sdio->ar->htc.endpoint[req->eid];
>> +	ath10k_htc_notify_tx_completion(ep, skb);
>> +	ath10k_sdio_free_bus_req(ar_sdio, req);
>> +}
>> +
>> +static void ath10k_sdio_write_async_work(struct work_struct *work)
>> +{
>> +	struct ath10k_sdio *ar_sdio;
>> +	struct ath10k_sdio_bus_request *req, *tmp_req;
>> +
>> +	ar_sdio = container_of(work, struct ath10k_sdio, wr_async_work);
>> +
>> +	spin_lock_bh(&ar_sdio->wr_async_lock);
>> +	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
>> +		list_del(&req->list);
>> +		spin_unlock_bh(&ar_sdio->wr_async_lock);
>> +		__ath10k_sdio_write_async(ar_sdio, req);
>> +		spin_lock_bh(&ar_sdio->wr_async_lock);
>> +	}
>> +	spin_unlock_bh(&ar_sdio->wr_async_lock);
>> +}
>> +
>> +static void ath10k_sdio_irq_handler(struct sdio_func *func)
>> +{
>> +	int status = 0;
>> +	unsigned long timeout;
>> +	struct ath10k_sdio *ar_sdio;
>> +	bool done = false;
>> +
>> +	ar_sdio = sdio_get_drvdata(func);
>> +	atomic_set(&ar_sdio->irq_handling, 1);
>> +
>> +	/* Release the host during interrupts so we can pick it back up when
>> +	 * we process commands.
>> +	 */
>> +	sdio_release_host(ar_sdio->func);
>> +
>> +	timeout = jiffies + ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ;
>> +	while (time_before(jiffies, timeout) && !done) {
>> +		status = ath10k_sdio_mbox_proc_pending_irqs(ar_sdio, &done);
>> +		if (status)
>> +			break;
>> +	}
>> +
>> +	sdio_claim_host(ar_sdio->func);
>> +
>> +	atomic_set(&ar_sdio->irq_handling, 0);
>> +	wake_up(&ar_sdio->irq_wq);
>> +
>> +	WARN_ON(status && status != -ECANCELED);
>> +}
> 
> Questionable use of an atomic variable again, looks like badly implement
> poor man's locking to me. And instead of wake_up() we should workqueues
> or threaded irqs (if sdio supports that).
> 
>> +
>> +static int ath10k_sdio_hif_disable_intrs(struct ath10k_sdio *ar_sdio)
>> +{
>> +	int ret;
>> +	struct ath10k_sdio_irq_enable_reg regs;
>> +	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
>> +
>> +	memset(&regs, 0, sizeof(regs));
>> +
>> +	ret = ath10k_sdio_read_write_sync(ar_sdio->ar,
>> +					  MBOX_INT_STATUS_ENABLE_ADDRESS,
>> +					  &regs.int_status_en, sizeof(regs),
>> +					  HIF_WR_SYNC_BYTE_INC);
>> +	if (ret) {
>> +		ath10k_err(ar_sdio->ar, "Unable to disable sdio interrupts\n");
>> +		return ret;
>> +	}
>> +
>> +	spin_lock_bh(&irq_data->lock);
>> +	irq_data->irq_en_reg = regs;
>> +	spin_unlock_bh(&irq_data->lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int ath10k_sdio_hif_power_up(struct ath10k *ar)
>> +{
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +	struct sdio_func *func = ar_sdio->func;
>> +	int ret = 0;
>> +
>> +	if (!ar_sdio->is_disabled)
>> +		return 0;
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power on\n");
>> +
>> +	sdio_claim_host(func);
>> +
>> +	ret = sdio_enable_func(func);
>> +	if (ret) {
>> +		ath10k_err(ar, "Unable to enable sdio func: %d)\n", ret);
>> +		sdio_release_host(func);
>> +		return ret;
>> +	}
>> +
>> +	sdio_release_host(func);
>> +
>> +	/* Wait for hardware to initialise. It should take a lot less than
>> +	 * 20 ms but let's be conservative here.
>> +	 */
>> +	msleep(20);
>> +
>> +	ar_sdio->is_disabled = false;
>> +
>> +	ret = ath10k_sdio_hif_disable_intrs(ar_sdio);
>> +
>> +	return ret;
>> +}
>> +
>> +static void ath10k_sdio_hif_power_down(struct ath10k *ar)
>> +{
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +	int ret;
>> +
>> +	if (ar_sdio->is_disabled)
>> +		return;
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio power off\n");
>> +
>> +	/* Disable the card */
>> +	sdio_claim_host(ar_sdio->func);
>> +	ret = sdio_disable_func(ar_sdio->func);
>> +	sdio_release_host(ar_sdio->func);
>> +
>> +	if (ret)
>> +		ath10k_dbg(ar, ATH10K_DBG_BOOT,
>> +			   "Unable to disable sdio: %d\n", ret);
> 
> Shouldn't this be ath10k_warn()?
> 
>> +
>> +	ar_sdio->is_disabled = true;
>> +}
>> +
>> +int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
>> +			  struct ath10k_hif_sg_item *items, int n_items)
>> +{
>> +	int i;
>> +	u32 address;
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +	struct ath10k_sdio_bus_request *bus_req;
>> +
>> +	bus_req = ath10k_sdio_alloc_busreq(ar_sdio);
>> +
>> +	if (WARN_ON_ONCE(!bus_req))
>> +		return -ENOMEM;
> 
> Is ath10k_warn() more approriate?
> 
>> +	for (i = 0; i < n_items; i++) {
>> +		bus_req->skb = items[i].transfer_context;
>> +		bus_req->request = HIF_WRITE;
>> +		bus_req->eid = pipe_id_to_eid(pipe_id);
>> +		/* Write TX data to the end of the mbox address space */
>> +		address = ar_sdio->mbox_addr[bus_req->eid] +
>> +			  ar_sdio->mbox_size[bus_req->eid] - bus_req->skb->len;
>> +		bus_req->address = address;
>> +		bus_req->len = CALC_TXRX_PADDED_LEN(ar_sdio, bus_req->skb->len);
>> +
>> +		spin_lock_bh(&ar_sdio->wr_async_lock);
>> +		list_add_tail(&bus_req->list, &ar_sdio->wr_asyncq);
>> +		spin_unlock_bh(&ar_sdio->wr_async_lock);
>> +	}
>> +
>> +	queue_work(ar_sdio->workqueue, &ar_sdio->wr_async_work);
>> +
>> +	return 0;
>> +}
>> +
>> +static int ath10k_sdio_hif_enable_intrs(struct ath10k_sdio *ar_sdio)
>> +{
>> +	struct ath10k_sdio_irq_enable_reg regs;
>> +	int status;
>> +	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
>> +
>> +	memset(&regs, 0, sizeof(regs));
>> +
>> +	/* Enable all but CPU interrupts */
>> +	regs.int_status_en = SM(0x01, MBOX_INT_STATUS_ENABLE_ERROR) |
>> +			     SM(0x01, MBOX_INT_STATUS_ENABLE_CPU) |
>> +			     SM(0x01, MBOX_INT_STATUS_ENABLE_COUNTER);
>> +
>> +	/* NOTE: There are some cases where HIF can do detection of
>> +	 * pending mbox messages which is disabled now.
>> +	 */
>> +	regs.int_status_en |= SM(0x01, MBOX_INT_STATUS_ENABLE_MBOX_DATA);
>> +
>> +	/* Set up the CPU Interrupt status Register */
>> +	regs.cpu_int_status_en = 0;
>> +
>> +	/* Set up the Error Interrupt status Register */
>> +	regs.err_int_status_en =
>> +		SM(0x01, MBOX_ERROR_STATUS_ENABLE_RX_UNDERFLOW) |
>> +		SM(0x01, MBOX_ERROR_STATUS_ENABLE_TX_OVERFLOW);
>> +
>> +	/* Enable Counter interrupt status register to get fatal errors for
>> +	 * debugging.
>> +	 */
>> +	regs.cntr_int_status_en = SM(ATH10K_SDIO_TARGET_DEBUG_INTR_MASK,
>> +				     MBOX_COUNTER_INT_STATUS_ENABLE_BIT);
>> +
>> +	status = ath10k_sdio_read_write_sync(ar_sdio->ar,
>> +					     MBOX_INT_STATUS_ENABLE_ADDRESS,
>> +					     &regs.int_status_en, sizeof(regs),
>> +					     HIF_WR_SYNC_BYTE_INC);
>> +	if (status) {
>> +		ath10k_err(ar_sdio->ar,
>> +			   "failed to update interrupt ctl reg err: %d\n",
>> +			   status);
>> +		return status;
>> +	}
>> +
>> +	spin_lock_bh(&irq_data->lock);
>> +	irq_data->irq_en_reg = regs;
>> +	spin_unlock_bh(&irq_data->lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int ath10k_sdio_hif_start(struct ath10k *ar)
>> +{
>> +	int ret;
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +	u32 addr, val;
>> +
>> +	addr = host_interest_item_address(HI_ITEM(hi_acs_flags));
>> +
>> +	ret = ath10k_sdio_hif_diag_read32(ar, addr, &val);
>> +	if (ret) {
>> +		ath10k_err(ar, "Unable to read diag mem: %d\n", ret);
>> +		goto out;
>> +	}
>> +
>> +	if (val & HI_ACS_FLAGS_SDIO_SWAP_MAILBOX_FW_ACK) {
>> +		ath10k_dbg(ar, ATH10K_DBG_SDIO,
>> +			   "Mailbox SWAP Service is enabled\n");
>> +		ar_sdio->swap_mbox = true;
>> +	}
>> +
>> +	/* eid 0 always uses the lower part of the extended mailbox address
>> +	 * space (ext_info[0].htc_ext_addr).
>> +	 */
>> +	ar_sdio->mbox_addr[0] = ar_sdio->mbox_info.ext_info[0].htc_ext_addr;
>> +	ar_sdio->mbox_size[0] = ar_sdio->mbox_info.ext_info[0].htc_ext_sz;
>> +
>> +	sdio_claim_host(ar_sdio->func);
>> +
>> +	/* Register the isr */
>> +	ret =  sdio_claim_irq(ar_sdio->func, ath10k_sdio_irq_handler);
>> +	if (ret) {
>> +		ath10k_err(ar, "Failed to claim sdio irq: %d\n", ret);
>> +		sdio_release_host(ar_sdio->func);
>> +		goto out;
>> +	}
>> +
>> +	sdio_release_host(ar_sdio->func);
>> +
>> +	ret = ath10k_sdio_hif_enable_intrs(ar_sdio);
>> +	if (ret)
>> +		ath10k_err(ar, "Failed to enable sdio interrupts: %d\n", ret);
>> +
>> +out:
>> +	return ret;
>> +}
>> +
>> +static bool ath10k_sdio_is_on_irq(struct ath10k *ar)
>> +{
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +
>> +	return !atomic_read(&ar_sdio->irq_handling);
>> +}
> 
> Yikes.
> 
>> +
>> +static void ath10k_sdio_irq_disable(struct ath10k *ar)
>> +{
>> +	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>> +	int ret;
>> +
>> +	sdio_claim_host(ar_sdio->func);
>> +
>> +	atomic_set(&ar_sdio->stopping, 1);
>> +
>> +	if (atomic_read(&ar_sdio->irq_handling)) {
>> +		sdio_release_host(ar_sdio->func);
>> +
>> +		ret = wait_event_interruptible(ar_sdio->irq_wq,
>> +					       ath10k_sdio_is_on_irq(ar));
>> +		if (ret)
>> +			return;
>> +
>> +		sdio_claim_host(ar_sdio->func);
>> +	}
> 
> There has to be a better way to implement this, now it feels that it's
> just reimplementing the wheel. We should have proper way to wait for
> interrupt handlers and workqueues to end etc.
> 
>> +	ret = sdio_release_irq(ar_sdio->func);
>> +	if (ret)
>> +		ath10k_err(ar, "Failed to release sdio irq: %d\n", ret);
>> +
>> +	sdio_release_host(ar_sdio->func);
>> +}
>> +
>> +static int ath10k_sdio_config(struct ath10k_sdio *ar_sdio)
>> +{
>> +	struct ath10k *ar = ar_sdio->ar;
>> +	struct sdio_func *func = ar_sdio->func;
>> +	unsigned char byte, asyncintdelay = 2;
>> +	int ret;
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_BOOT, "SDIO configuration\n");
>> +
>> +	sdio_claim_host(func);
>> +
>> +	byte = 0;
>> +	ret = ath10k_sdio_func0_cmd52_rd_byte(func->card,
>> +					      SDIO_CCCR_DRIVE_STRENGTH,
>> +					      &byte);
>> +
>> +	byte = (byte & (~(SDIO_DRIVE_DTSx_MASK << SDIO_DRIVE_DTSx_SHIFT))) |
>> +		SDIO_DTSx_SET_TYPE_D;
> 
> FIELD_PREP(). There are lots of places where that an be used.
> 
>> +static void ath10k_sdio_write32(struct ath10k *ar, u32 offset, u32 value)
>> +{
>> +}
>> +
>> +static u32 ath10k_sdio_read32(struct ath10k *ar, u32 offset)
>> +{
>> +	return 0;
>> +}
> 
> Somekind of FIXME/TODO comments for write32() and read32()? What
> functionality are we going to miss when we don't implement these?
> 
>> +static void ath10k_sdio_hif_send_complete_check(struct ath10k *ar,
>> +						u8 pipe, int force)
>> +{
>> +}
>> +
>> +static u16 ath10k_sdio_hif_get_free_queue_number(struct ath10k *ar, u8 pipe)
>> +{
>> +	return 0;
>> +}
> 
> Similar comments here also.
> 
>> +
>> +static const struct ath10k_hif_ops ath10k_sdio_hif_ops = {
>> +	.tx_sg			= ath10k_sdio_hif_tx_sg,
>> +	.diag_read		= ath10k_sdio_hif_diag_read,
>> +	.diag_write		= ath10k_sdio_diag_write_mem,
>> +	.exchange_bmi_msg	= ath10k_sdio_hif_exchange_bmi_msg,
>> +	.start			= ath10k_sdio_hif_start,
>> +	.stop			= ath10k_sdio_hif_stop,
>> +	.map_service_to_pipe	= ath10k_sdio_hif_map_service_to_pipe,
>> +	.get_default_pipe	= ath10k_sdio_hif_get_default_pipe,
>> +	.send_complete_check	= ath10k_sdio_hif_send_complete_check,
>> +	.get_free_queue_number	= ath10k_sdio_hif_get_free_queue_number,
>> +	.power_up		= ath10k_sdio_hif_power_up,
>> +	.power_down		= ath10k_sdio_hif_power_down,
>> +	.read32			= ath10k_sdio_read32,
>> +	.write32		= ath10k_sdio_write32,
>> +#ifdef CONFIG_PM
>> +	.suspend		= ath10k_sdio_hif_suspend,
>> +	.resume			= ath10k_sdio_hif_resume,
>> +#endif
>> +	.fetch_cal_eeprom	= ath10k_sdio_hif_fetch_cal_eeprom,
>> +};
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +
>> +/* Empty handlers so that mmc subsystem doesn't remove us entirely during
>> + * suspend. We instead follow cfg80211 suspend/resume handlers.
>> + */
>> +static int ath10k_sdio_pm_suspend(struct device *device)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int ath10k_sdio_pm_resume(struct device *device)
>> +{
>> +	return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(ath10k_sdio_pm_ops, ath10k_sdio_pm_suspend,
>> +			 ath10k_sdio_pm_resume);
>> +
>> +#define ATH10K_SDIO_PM_OPS (&ath10k_sdio_pm_ops)
>> +
>> +#else
>> +
>> +#define ATH10K_SDIO_PM_OPS NULL
>> +
>> +#endif /* CONFIG_PM_SLEEP */
>> +
>> +static int ath10k_sdio_probe(struct sdio_func *func,
>> +			     const struct sdio_device_id *id)
>> +{
>> +	int ret;
>> +	struct ath10k_sdio *ar_sdio;
>> +	struct ath10k *ar;
>> +	int i;
>> +	enum ath10k_hw_rev hw_rev;
>> +
>> +	hw_rev = ATH10K_HW_QCA6174;
> 
> This needs a comment, at least to remind if ever add something else than
> QCA6174 based SDIO design.
> 
>> +
>> +	ar = ath10k_core_create(sizeof(*ar_sdio), &func->dev, ATH10K_BUS_SDIO,
>> +				hw_rev, &ath10k_sdio_hif_ops);
>> +	if (!ar) {
>> +		dev_err(&func->dev, "failed to allocate core\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_BOOT,
>> +		   "sdio new func %d vendor 0x%x device 0x%x block 0x%x/0x%x\n",
>> +		   func->num, func->vendor, func->device,
>> +		   func->max_blksize, func->cur_blksize);
>> +
>> +	ar_sdio = ath10k_sdio_priv(ar);
>> +
>> +	ar_sdio->dma_buffer = kzalloc(ATH10K_HIF_DMA_BUFFER_SIZE, GFP_KERNEL);
>> +	if (!ar_sdio->dma_buffer) {
>> +		ret = -ENOMEM;
>> +		goto err_core_destroy;
>> +	}
>> +
>> +	ar_sdio->func = func;
>> +	sdio_set_drvdata(func, ar_sdio);
>> +
>> +	ar_sdio->is_disabled = true;
>> +	ar_sdio->ar = ar;
>> +
>> +	spin_lock_init(&ar_sdio->lock);
>> +	spin_lock_init(&ar_sdio->wr_async_lock);
>> +	spin_lock_init(&ar_sdio->irq_data.lock);
>> +	mutex_init(&ar_sdio->dma_buffer_mutex);
>> +
>> +	INIT_LIST_HEAD(&ar_sdio->bus_req_freeq);
>> +	INIT_LIST_HEAD(&ar_sdio->wr_asyncq);
>> +
>> +	INIT_WORK(&ar_sdio->wr_async_work, ath10k_sdio_write_async_work);
>> +	ar_sdio->workqueue = create_singlethread_workqueue("ath10k_sdio_wq");
>> +	if (!ar_sdio->workqueue)
>> +		goto err;
>> +
>> +	init_waitqueue_head(&ar_sdio->irq_wq);
>> +
>> +	for (i = 0; i < ATH10K_SDIO_BUS_REQUEST_MAX_NUM; i++)
>> +		ath10k_sdio_free_bus_req(ar_sdio, &ar_sdio->bus_req[i]);
>> +
>> +	ar->dev_id = id->device;
>> +	ar->id.vendor = id->vendor;
>> +	ar->id.device = id->device;
>> +
>> +	ath10k_sdio_set_mbox_info(ar_sdio);
>> +
>> +	ret = ath10k_sdio_config(ar_sdio);
>> +	if (ret) {
>> +		ath10k_err(ar, "Failed to config sdio: %d\n", ret);
>> +		goto err;
>> +	}
>> +
>> +	ret = ath10k_core_register(ar, 0/*chip_id is not applicaple for SDIO*/);
>> +	if (ret) {
>> +		ath10k_err(ar, "failed to register driver core: %d\n", ret);
>> +		goto err;
>> +	}
> 
> I would assume that chip_id is applicaple also with SDIO, there has to
> be a register where to get it. Also this kind of comment style is
> preferred:
> 
> /* TODO: don't know yet how to get chip_id with SDIO */
> chip_id = 0;
> 
> ret = ath10k_core_register(ar, chip_id);
> 
>> +
>> +	return ret;
>> +
>> +err:
>> +	kfree(ar_sdio->dma_buffer);
>> +err_core_destroy:
>> +	ath10k_core_destroy(ar);
>> +
>> +	return ret;
>> +}
>> +
>> +static void ath10k_sdio_remove(struct sdio_func *func)
>> +{
>> +	struct ath10k_sdio *ar_sdio;
>> +	struct ath10k *ar;
>> +
>> +	ar_sdio = sdio_get_drvdata(func);
>> +	ar = ar_sdio->ar;
>> +
>> +	ath10k_dbg(ar, ATH10K_DBG_BOOT,
>> +		   "sdio removed func %d vendor 0x%x device 0x%x\n",
>> +		   func->num, func->vendor, func->device);
>> +
>> +	(void)ath10k_sdio_hif_disable_intrs(ar_sdio);
>> +	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[] = {
>> +	{SDIO_DEVICE(ATH10K_MANUFACTURER_CODE,
>> +		     (ATH10K_MANUFACTURER_ID_AR6005_BASE | 0xA))},
>> +	{},
>> +};
> 
> I suspect there's a more sensible way to create the device table than
> this, just no time to check it now. Anyone know?
> 
> The naming "ath10k manufacturer" is also wrong, it should be QCA or
> Qualcomm.
>