[PATCH 0/7] ath10k: remove ath10k_sdio_io()

[PATCH 0/7] ath10k: remove ath10k_sdio_io()

[Kalle Valo]

Using ath10k_sdio_io() and ath10k_sdio_read_write_sync() are just
confusing. Refactor the code by following the plan outlined
below.

I'm planning to fold all these seven patches into Erik's patch
adding sdio.c[1] so I didn't write any commit logs. I'm just
submitting these separately to make it easier to review the
changes.

Kalle

ath10k_sdio_read_write_sync():
X HIF_RD_SYNC_BLOCK_FIX	-> ath10k_sdio_readsb()
X HIF_RD_SYNC_BYTE_INC	-> ath10k_sdio_read(), ath10k_sdio_read32()
X HIF_WR_SYNC_BYTE_FIX	-> ath10k_sdio_writesb32()
X HIF_WR_SYNC_BYTE_INC	-> ath10k_sdio_write()

ath10k_sdio_prep_async_req():
X HIF_WRITE		-> HIF_WR_SYNC_BYTE_INC -> ath10k_sdio_write()

ath10k_sdio_read/write32():
sdio_readl()/sdio_writel() (those use endian macros)

ath10k_sdio_writesb32():
kmalloc()
sdio_writesb(len)
kfree()

ath10k_sdio_readsb():
len = round_down(len, ar_sdio->mbox_info.block_size);
sdio_readsb(len)

ath10k_sdio_read():
sdio_memcpy_fromio()

ath10k_sdio_write():
sdio_memcpy_toio()


[1] https://patchwork.kernel.org/patch/9626001/

---

Kalle Valo (7):
      ath10k: add ath10k_sdio_write32/read32()
      ath10k: add ath10k_sdio_writesb()
      ath10k: add ath10k_sdio_read()
      ath10k: add ath10k_sdio_write()
      ath10k: add ath10k_sdio_readsb()
      ath10k: convert __ath10k_sdio_write_async() to use ath10k_sdio_write()
      ath10k: remove unused sdio wrappers


 drivers/net/wireless/ath/ath10k/sdio.c |  419 +++++++++++++++++---------------
 drivers/net/wireless/ath/ath10k/sdio.h |   40 ---
 2 files changed, 219 insertions(+), 240 deletions(-)


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 2/7] ath10k: add ath10k_sdio_writesb()

[Kalle Valo]

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c |   85 +++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index c7bc86e5063f..56728d44dd12 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -287,6 +287,39 @@ static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
 	return ret;
 }
 
+static int ath10k_sdio_writesb32(struct ath10k *ar, u32 addr, u32 val)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	__le32 *buf;
+	int ret;
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	*buf = cpu_to_le32(val);
+
+	sdio_claim_host(func);
+
+	ret = sdio_writesb(func, addr, buf, sizeof(*buf));
+	if (ret) {
+		ath10k_warn(ar, "failed to write value 0x%x to fixed sb address 0x%x: %d\n",
+			    val, addr, ret);
+		goto out;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio writesb32 addr 0x%x val 0x%x\n",
+		   addr, val);
+
+out:
+	sdio_release_host(func);
+
+	kfree(buf);
+
+	return ret;
+}
+
 static int ath10k_sdio_read32(struct ath10k *ar, u32 addr, u32 *val)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
@@ -709,21 +742,16 @@ static int ath10k_sdio_mbox_proc_err_intr(struct ath10k *ar)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
-	u8 error_int_status, *reg_buf;
+	u8 error_int_status;
 	int ret;
 
-	reg_buf = kzalloc(4, GFP_KERNEL);
-	if (!reg_buf)
-		return -ENOMEM;
-
 	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio error interrupt\n");
 
 	error_int_status = irq_data->irq_proc_reg->error_int_status & 0x0F;
 	if (!error_int_status) {
 		ath10k_warn(ar, "invalid error interrupt status: 0x%x\n",
 			    error_int_status);
-		ret = -EIO;
-		goto out;
+		return -EIO;
 	}
 
 	ath10k_dbg(ar, ATH10K_DBG_SDIO,
@@ -744,29 +772,25 @@ static int ath10k_sdio_mbox_proc_err_intr(struct ath10k *ar)
 	/* Clear the interrupt */
 	irq_data->irq_proc_reg->error_int_status &= ~error_int_status;
 
+	/* FIXME: is this correct? originally it was "reg_buf[0] =
+	 * error_int_status" and not sure if this is the same anymore */
 	/* set W1C value to clear the interrupt, this hits the register first */
-	reg_buf[0] = error_int_status;
-
-	ret = ath10k_sdio_read_write_sync(ar,
-					  MBOX_ERROR_INT_STATUS_ADDRESS,
-					  reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
+	ret = ath10k_sdio_writesb32(ar, MBOX_ERROR_INT_STATUS_ADDRESS,
+				    error_int_status);
 	if (ret) {
 		ath10k_warn(ar, "unable to write to error int status address: %d\n",
 			    ret);
-		goto out;
+		return ret;
 	}
 
-out:
-	kfree(reg_buf);
-
-	return ret;
+	return 0;
 }
 
 static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k *ar)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	struct ath10k_sdio_irq_data *irq_data = &ar_sdio->irq_data;
-	u8 cpu_int_status, *reg_buf;
+	u8 cpu_int_status;
 	int ret;
 
 	mutex_lock(&irq_data->mtx);
@@ -781,32 +805,23 @@ static int ath10k_sdio_mbox_proc_cpu_intr(struct ath10k *ar)
 	/* 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 ,
+	/* FIXME: is this correct? originally it was "reg_buf[0] =
+	 * cpu_int_status" and not sure if this is the same anymore */
+	/* 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 = kzalloc(4, GFP_KERNEL);
-	if (!reg_buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	/* set W1C value to clear the interrupt, this hits the register first */
-	reg_buf[0] = cpu_int_status;
-
-	ret = ath10k_sdio_read_write_sync(ar,
-					  MBOX_CPU_INT_STATUS_ADDRESS,
-					  reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
+	ret = ath10k_sdio_writesb32(ar, MBOX_CPU_INT_STATUS_ADDRESS,
+				    cpu_int_status);
 	if (ret) {
 		ath10k_warn(ar, "unable to write to cpu interrupt status address: %d\n",
 			    ret);
-		goto out_free;
+		goto out;
 	}
 
-out_free:
-	kfree(reg_buf);
-
 out:
 	mutex_unlock(&irq_data->mtx);
 	return ret;


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 3/7] ath10k: add ath10k_sdio_read()

[Kalle Valo]

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c |   40 +++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 56728d44dd12..bbd8317de861 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -343,6 +343,31 @@ static int ath10k_sdio_read32(struct ath10k *ar, u32 addr, u32 *val)
 	return ret;
 }
 
+static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	int ret;
+
+	sdio_claim_host(func);
+
+	ret = sdio_memcpy_fromio(func, buf, addr, len);
+	if (ret) {
+		ath10k_warn(ar, "failed to read from address 0x%x: %d\n",
+			    addr, ret);
+		goto out;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio read addr 0x%x buf 0x%p len %zu\n",
+		   addr, buf, len);
+	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL, "sdio read ", buf, len);
+
+out:
+	sdio_release_host(func);
+
+	return ret;
+}
+
 /* HIF mbox functions */
 
 static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
@@ -859,11 +884,8 @@ static int ath10k_sdio_mbox_read_int_status(struct ath10k *ar,
 	 * will yield us the value of different int status
 	 * registers and the lookahead registers.
 	 */
-	ret = ath10k_sdio_read_write_sync(ar,
-					  MBOX_HOST_INT_STATUS_ADDRESS,
-					  (u8 *)irq_proc_reg,
-					  sizeof(*irq_proc_reg),
-					  HIF_RD_SYNC_BYTE_INC);
+	ret = ath10k_sdio_read(ar, MBOX_HOST_INT_STATUS_ADDRESS,
+			       irq_proc_reg, sizeof(*irq_proc_reg));
 	if (ret)
 		goto out;
 
@@ -1175,9 +1197,7 @@ static int ath10k_sdio_bmi_exchange_msg(struct ath10k *ar,
 
 	/* We always read from the start of the mbox address */
 	addr = ar_sdio->mbox_info.htc_addr;
-	ret = ath10k_sdio_read_write_sync(ar, addr, ar_sdio->bmi_buf,
-					  *resp_len,
-					  HIF_RD_SYNC_BYTE_INC);
+	ret = ath10k_sdio_read(ar, addr, ar_sdio->bmi_buf, *resp_len);
 	if (ret) {
 		ath10k_warn(ar,
 			    "unable to read the bmi data from the device: %d\n",
@@ -1538,9 +1558,7 @@ static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
 	}
 
 	/* read the data */
-	ret = ath10k_sdio_read_write_sync(ar, MBOX_WINDOW_DATA_ADDRESS,
-					  (u8 *)buf, buf_len,
-					  HIF_RD_SYNC_BYTE_INC);
+	ret = ath10k_sdio_read(ar, MBOX_WINDOW_DATA_ADDRESS, buf, buf_len);
 	if (ret) {
 		ath10k_warn(ar, "failed to read from mbox window data addrress: %d\n",
 			    ret);


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 4/7] ath10k: add ath10k_sdio_write()

[Kalle Valo]

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c |   48 +++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index bbd8317de861..beb1749672f0 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -368,6 +368,34 @@ static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf, size_t len)
 	return ret;
 }
 
+static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_t len)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	int ret;
+
+	sdio_claim_host(func);
+
+	/* For some reason toio() doesn't have const for the buffer, need
+	 * an ugly hack to workaround that.
+	 */
+	ret = sdio_memcpy_toio(func, addr, (void *)buf, len);
+	if (ret) {
+		ath10k_warn(ar, "failed to write to address 0x%x: %d\n",
+			    addr, ret);
+		goto out;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio write addr 0x%x buf 0x%p len %zu\n",
+		   addr, buf, len);
+	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL, "sdio write ", buf, len);
+
+out:
+	sdio_release_host(func);
+
+	return ret;
+}
+
 /* HIF mbox functions */
 
 static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
@@ -1131,9 +1159,7 @@ static int ath10k_sdio_bmi_exchange_msg(struct ath10k *ar,
 		addr = ar_sdio->mbox_info.htc_addr;
 
 		memcpy(ar_sdio->bmi_buf, req, req_len);
-		ret = ath10k_sdio_read_write_sync(ar, addr, ar_sdio->bmi_buf,
-						  req_len,
-						  HIF_WR_SYNC_BYTE_INC);
+		ret = ath10k_sdio_write(ar, addr, ar_sdio->bmi_buf, req_len);
 		if (ret) {
 			ath10k_warn(ar,
 				    "unable to send the bmi data to the device: %d\n",
@@ -1361,10 +1387,8 @@ static int ath10k_sdio_hif_disable_intrs(struct ath10k *ar)
 	mutex_lock(&irq_data->mtx);
 
 	memset(regs, 0, sizeof(*regs));
-	ret = ath10k_sdio_read_write_sync(ar,
-					  MBOX_INT_STATUS_ENABLE_ADDRESS,
-					  &regs->int_status_en, sizeof(*regs),
-					  HIF_WR_SYNC_BYTE_INC);
+	ret = ath10k_sdio_write(ar, MBOX_INT_STATUS_ENABLE_ADDRESS,
+				&regs->int_status_en, sizeof(*regs));
 	if (ret)
 		ath10k_warn(ar, "unable to disable sdio interrupts: %d\n", ret);
 
@@ -1498,10 +1522,8 @@ static int ath10k_sdio_hif_enable_intrs(struct ath10k *ar)
 		FIELD_PREP(MBOX_COUNTER_INT_STATUS_ENABLE_BIT_MASK,
 			   ATH10K_SDIO_TARGET_DEBUG_INTR_MASK);
 
-	ret = ath10k_sdio_read_write_sync(ar,
-					  MBOX_INT_STATUS_ENABLE_ADDRESS,
-					  &regs->int_status_en, sizeof(*regs),
-					  HIF_WR_SYNC_BYTE_INC);
+	ret = ath10k_sdio_write(ar, MBOX_INT_STATUS_ENABLE_ADDRESS,
+				&regs->int_status_en, sizeof(*regs));
 	if (ret)
 		ath10k_warn(ar,
 			    "failed to update mbox interrupt status register : %d\n",
@@ -1596,9 +1618,7 @@ static int ath10k_sdio_hif_diag_write_mem(struct ath10k *ar, u32 address,
 	int ret;
 
 	/* set write data */
-	ret = ath10k_sdio_read_write_sync(ar, MBOX_WINDOW_DATA_ADDRESS,
-					  (u8 *)data, nbytes,
-					  HIF_WR_SYNC_BYTE_INC);
+	ret = ath10k_sdio_write(ar, MBOX_WINDOW_DATA_ADDRESS, data, nbytes);
 	if (ret) {
 		ath10k_warn(ar,
 			    "failed to write 0x%p to mbox window data addrress: %d\n",


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 5/7] ath10k: add ath10k_sdio_readsb()

[Kalle Valo]

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c |   34 +++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index beb1749672f0..301355228cfa 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -396,6 +396,33 @@ static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const void *buf, size_
 	return ret;
 }
 
+static int ath10k_sdio_readsb(struct ath10k *ar, u32 addr, void *buf, size_t len)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	int ret;
+
+	sdio_claim_host(func);
+
+	len = round_down(len, ar_sdio->mbox_info.block_size);
+
+	ret = sdio_readsb(func, buf, addr, len);
+	if (ret) {
+		ath10k_warn(ar, "failed to read from fixed (sb) address 0x%x: %d\n",
+			    addr, ret);
+		goto out;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio readsb addr 0x%x buf 0x%p len %zu\n",
+		   addr, buf, len);
+	ath10k_dbg_dump(ar, ATH10K_DBG_SDIO_DUMP, NULL, "sdio readsb ", buf, len);
+
+out:
+	sdio_release_host(func);
+
+	return ret;
+}
+
 /* HIF mbox functions */
 
 static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
@@ -645,11 +672,8 @@ static int ath10k_sdio_mbox_rx_packet(struct ath10k *ar,
 	struct sk_buff *skb = pkt->skb;
 	int ret;
 
-	ret = ath10k_sdio_read_write_sync(ar,
-					  ar_sdio->mbox_info.htc_addr,
-					  skb->data, pkt->alloc_len,
-					  HIF_RD_SYNC_BLOCK_FIX);
-
+	ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
+				 skb->data, pkt->alloc_len);
 	pkt->status = ret;
 	if (!ret)
 		skb_put(skb, pkt->act_len);


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 6/7] ath10k: convert __ath10k_sdio_write_async() to use ath10k_sdio_write()

[Kalle Valo]

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c |    9 +++++----
 drivers/net/wireless/ath/ath10k/sdio.h |    2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 301355228cfa..8ea1629c7bae 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -1304,9 +1304,11 @@ static void __ath10k_sdio_write_async(struct ath10k *ar,
 	int ret;
 
 	skb = req->skb;
-	ret = ath10k_sdio_read_write_sync(ar, req->address,
-					  skb->data, skb->len,
-					  req->request);
+	ret = ath10k_sdio_write(ar, req->address, skb->data, skb->len);
+	if (ret)
+		ath10k_warn(ar, "failed to write skb to 0x%x asynchronously: %d",
+			    req->address, ret);
+
 	if (req->htc_msg) {
 		ep = &ar->htc.endpoint[req->eid];
 		ath10k_htc_notify_tx_completion(ep, skb);
@@ -1355,7 +1357,6 @@ static int ath10k_sdio_prep_async_req(struct ath10k *ar, u32 addr,
 	}
 
 	bus_req->skb = skb;
-	bus_req->request = HIF_WRITE;
 	bus_req->eid = eid;
 	bus_req->address = addr;
 	bus_req->htc_msg = htc_msg;
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index d1e4f78ddaeb..1bce56447e53 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -110,7 +110,7 @@ struct ath10k_sdio_bus_request {
 
 	/* sdio address */
 	u32 address;
-	u32 request;
+
 	struct sk_buff *skb;
 	enum ath10k_htc_ep_id eid;
 	int status;


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 7/7] ath10k: remove unused sdio wrappers

[Kalle Valo]

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/sdio.c |   44 --------------------------------
 drivers/net/wireless/ath/ath10k/sdio.h |   35 -------------------------
 2 files changed, 79 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 8ea1629c7bae..a5ed4282b3b3 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -219,50 +219,6 @@ static int ath10k_sdio_config(struct ath10k *ar)
 	return ret;
 }
 
-static int ath10k_sdio_io(struct ath10k *ar, u32 request, u32 addr,
-			  u8 *buf, u32 len)
-{
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct sdio_func *func = ar_sdio->func;
-	int ret;
-
-	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, "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 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);
-
-	if (request & HIF_BLOCK_BASIS)
-		len = round_down(len, ar_sdio->mbox_info.block_size);
-
-	return ath10k_sdio_io(ar, request, addr, buf, len);
-}
-
 static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index 1bce56447e53..62d2699123ec 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -136,41 +136,6 @@ struct ath10k_sdio_rx_data {
 	int status;
 };
 
-/* direction of transfer (read/write) */
-#define HIF_READ                    0x00000001
-#define HIF_WRITE                   0x00000002
-
-/*     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
-
-/*     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_WR_SYNC_BYTE_FIX					\
-	(HIF_WRITE | HIF_BYTE_BASIS | HIF_FIXED_ADDRESS)
-
-#define HIF_WR_SYNC_BYTE_INC					\
-	(HIF_WRITE | HIF_BYTE_BASIS | HIF_INCREMENTAL_ADDRESS)
-
-#define HIF_RD_SYNC_BYTE_INC						\
-	(HIF_READ | HIF_BYTE_BASIS | HIF_INCREMENTAL_ADDRESS)
-
-#define HIF_RD_SYNC_BLOCK_FIX						\
-	(HIF_READ | HIF_BLOCK_BASIS | HIF_FIXED_ADDRESS)
-
 struct ath10k_sdio_irq_proc_regs {
 	u8 host_int_status;
 	u8 cpu_int_status;


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 0/7] ath10k: remove ath10k_sdio_io()

[Erik Stromdahl]

I can't find patch 1/7...

On 2017-04-05 09:06, Kalle Valo wrote:
> Using ath10k_sdio_io() and ath10k_sdio_read_write_sync() are just
> confusing. Refactor the code by following the plan outlined
> below.
>
> I'm planning to fold all these seven patches into Erik's patch
> adding sdio.c[1] so I didn't write any commit logs. I'm just
> submitting these separately to make it easier to review the
> changes.
>
> Kalle
>
> ath10k_sdio_read_write_sync():
> X HIF_RD_SYNC_BLOCK_FIX	-> ath10k_sdio_readsb()
> X HIF_RD_SYNC_BYTE_INC	-> ath10k_sdio_read(), ath10k_sdio_read32()
> X HIF_WR_SYNC_BYTE_FIX	-> ath10k_sdio_writesb32()
> X HIF_WR_SYNC_BYTE_INC	-> ath10k_sdio_write()
>
> ath10k_sdio_prep_async_req():
> X HIF_WRITE		-> HIF_WR_SYNC_BYTE_INC -> ath10k_sdio_write()
>
> ath10k_sdio_read/write32():
> sdio_readl()/sdio_writel() (those use endian macros)
>
> ath10k_sdio_writesb32():
> kmalloc()
> sdio_writesb(len)
> kfree()
>
> ath10k_sdio_readsb():
> len = round_down(len, ar_sdio->mbox_info.block_size);
> sdio_readsb(len)
>
> ath10k_sdio_read():
> sdio_memcpy_fromio()
>
> ath10k_sdio_write():
> sdio_memcpy_toio()
>
>
> [1] https://patchwork.kernel.org/patch/9626001/
>
> ---
>
> Kalle Valo (7):
>       ath10k: add ath10k_sdio_write32/read32()
>       ath10k: add ath10k_sdio_writesb()
>       ath10k: add ath10k_sdio_read()
>       ath10k: add ath10k_sdio_write()
>       ath10k: add ath10k_sdio_readsb()
>       ath10k: convert __ath10k_sdio_write_async() to use ath10k_sdio_write()
>       ath10k: remove unused sdio wrappers
>
>
>  drivers/net/wireless/ath/ath10k/sdio.c |  419 +++++++++++++++++---------------
>  drivers/net/wireless/ath/ath10k/sdio.h |   40 ---
>  2 files changed, 219 insertions(+), 240 deletions(-)
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 0/7] ath10k: remove ath10k_sdio_io()

[Kalle Valo]

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

> I can't find patch 1/7...

Odd, apparently it didn't get delivered to ath10k list but did get to
linux-wireless. You can find the patch from linux-wireless patchwork:

https://patchwork.kernel.org/patch/9663097/

Also copying it below.

Kalle

Author: Kalle Valo <kvalo@qca.qualcomm.com>
Date:   Tue Mar 28 10:27:00 2017 +0300

    ath10k: add ath10k_sdio_write32/read32()
    
    Also remove ath10k_sdio_hif_set_addrwin_reg(), after the conversion it looked
    useless.
    
    Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 0dced846035b..c7bc86e5063f 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -263,6 +263,53 @@ static int ath10k_sdio_read_write_sync(struct ath10k *ar, u32 addr, u8 *buf,
 	return ath10k_sdio_io(ar, request, addr, buf, len);
 }
 
+static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	int ret;
+
+	sdio_claim_host(func);
+
+	sdio_writel(func, val, addr, &ret);
+	if (ret) {
+		ath10k_warn(ar, "failed to write 0x%x to address 0x%x: %d\n",
+			    val, addr, ret);
+		goto out;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio write32 addr 0x%x val 0x%x\n",
+		   addr, val);
+
+out:
+	sdio_release_host(func);
+
+	return ret;
+}
+
+static int ath10k_sdio_read32(struct ath10k *ar, u32 addr, u32 *val)
+{
+	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct sdio_func *func = ar_sdio->func;
+	int ret;
+
+	sdio_claim_host(func);
+	*val = sdio_readl(func, addr, &ret);
+	if (ret) {
+		ath10k_warn(ar, "failed to read from address 0x%x: %d\n",
+			    addr, ret);
+		goto out;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio read32 addr 0x%x val 0x%x\n",
+		   addr, *val);
+
+out:
+	sdio_release_host(func);
+
+	return ret;
+}
+
 /* HIF mbox functions */
 
 static int ath10k_sdio_mbox_rx_process_packet(struct ath10k *ar,
@@ -617,27 +664,19 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar,
 
 static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k *ar)
 {
-	u32 *dummy;
+	u32 val;
 	int ret;
 
-	dummy = kzalloc(sizeof(*dummy), GFP_KERNEL);
-	if (!dummy)
-		return -ENOMEM;
-
 	/* TODO: Add firmware crash handling */
 	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, sizeof(*dummy),
-					  HIF_RD_SYNC_BYTE_INC);
+	ret = ath10k_sdio_read32(ar, MBOX_COUNT_DEC_ADDRESS, &val);
 	if (ret)
 		ath10k_warn(ar, "failed to clear debug interrupt: %d\n", ret);
 
-	kfree(dummy);
-
 	return ret;
 }
 
@@ -972,90 +1011,70 @@ static void ath10k_sdio_set_mbox_info(struct ath10k *ar)
 
 static int ath10k_sdio_bmi_credits(struct ath10k *ar)
 {
-	int ret;
-	u32 addr, *cmd_credits;
+	u32 addr, cmd_credits;
 	unsigned long timeout;
-
-	cmd_credits = kzalloc(sizeof(*cmd_credits), GFP_KERNEL);
-	if (!cmd_credits)
-		return -ENOMEM;
+	int ret;
 
 	/* 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) {
+	cmd_credits = 0;
+
+	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);
+		ret = ath10k_sdio_read32(ar, addr, &cmd_credits);
 		if (ret) {
 			ath10k_warn(ar,
 				    "unable to decrement the command credit count register: %d\n",
 				    ret);
-			goto out;
+			return ret;
 		}
 
 		/* The counter is only 8 bits.
 		 * Ignore anything in the upper 3 bytes
 		 */
-		*cmd_credits &= 0xFF;
+		cmd_credits &= 0xFF;
 	}
 
-	if (!*cmd_credits) {
+	if (!cmd_credits) {
 		ath10k_warn(ar, "bmi communication timeout\n");
-		ret = -ETIMEDOUT;
-		goto out;
+		return -ETIMEDOUT;
 	}
 
-	ret = 0;
-
-out:
-	kfree(cmd_credits);
-
-	return ret;
+	return 0;
 }
 
 static int ath10k_sdio_bmi_get_rx_lookahead(struct ath10k *ar)
 {
 	unsigned long timeout;
-	u32 *rx_word;
+	u32 rx_word;
 	int ret;
 
-	rx_word = kzalloc(sizeof(*rx_word), GFP_KERNEL);
-	if (!rx_word)
-		return -ENOMEM;
-
 	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);
+	rx_word = 0;
+
+	while ((time_before(jiffies, timeout)) && !rx_word) {
+		ret = ath10k_sdio_read32(ar,
+					 MBOX_HOST_INT_STATUS_ADDRESS,
+					 &rx_word);
 		if (ret) {
 			ath10k_warn(ar, "unable to read RX_LOOKAHEAD_VALID: %d\n", ret);
-			goto out;
+			return ret;
 		}
 
 		 /* all we really want is one bit */
-		*rx_word &= 1;
+		rx_word &= 1;
 	}
 
-	if (!*rx_word) {
+	if (!rx_word) {
 		ath10k_warn(ar, "bmi_recv_buf FIFO empty\n");
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
-out:
-	kfree(rx_word);
-
 	return ret;
 }
 
@@ -1464,69 +1483,29 @@ static int ath10k_sdio_hif_enable_intrs(struct ath10k *ar)
 
 static int ath10k_sdio_hif_set_mbox_sleep(struct ath10k *ar, bool enable_sleep)
 {
-	u32 *val;
+	u32 val;
 	int ret;
 
-	val = kzalloc(sizeof(*val), GFP_KERNEL);
-	if (!val)
-		return -ENOMEM;
-
-	ret = ath10k_sdio_read_write_sync(ar, FIFO_TIMEOUT_AND_CHIP_CONTROL,
-					  (u8 *)val, sizeof(*val),
-					  HIF_RD_SYNC_BYTE_INC);
+	ret = ath10k_sdio_read32(ar, FIFO_TIMEOUT_AND_CHIP_CONTROL, &val);
 	if (ret) {
 		ath10k_warn(ar, "failed to read fifo/chip control register: %d\n",
 			    ret);
-		goto out;
+		return ret;
 	}
 
 	if (enable_sleep)
-		*val &= FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_OFF;
+		val &= FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_OFF;
 	else
-		*val |= FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_ON;
-
-	ret = ath10k_sdio_read_write_sync(ar, FIFO_TIMEOUT_AND_CHIP_CONTROL,
-					  (u8 *)val, sizeof(*val),
-					  HIF_WR_SYNC_BYTE_INC);
-	if (ret)
-		goto out;
-
-	ret = 0;
-
-out:
-	kfree(val);
-
-	return ret;
-}
-
-/* set the window address register (using 4-byte register access ). */
-static int ath10k_sdio_hif_set_addrwin_reg(struct ath10k *ar, u32 reg_addr,
-					   u32 addr)
-{
-	u32 *tmp_addr;
-	int ret;
-
-	tmp_addr = kmalloc(sizeof(addr), GFP_KERNEL);
-	if (!tmp_addr)
-		return -ENOMEM;
-
-	*tmp_addr = addr;
-	ret = ath10k_sdio_read_write_sync(ar, reg_addr, (u8 *)tmp_addr,
-					  4, HIF_WR_SYNC_BYTE_INC);
+		val |= FIFO_TIMEOUT_AND_CHIP_CONTROL_DISABLE_SLEEP_ON;
 
+	ret = ath10k_sdio_write32(ar, FIFO_TIMEOUT_AND_CHIP_CONTROL, val);
 	if (ret) {
-		ath10k_warn(ar,
-			    "failed to write 0x%x to window reg: %d\n",
-			    reg_addr, ret);
-		goto out;
+		ath10k_warn(ar, "failed to write to FIFO_TIMEOUT_AND_CHIP_CONTROL: %d",
+			    ret);
+		return ret;
 	}
 
-	ret = 0;
-
-out:
-	kfree(tmp_addr);
-
-	return ret;
+	return 0;
 }
 
 /* HIF diagnostics */
@@ -1537,11 +1516,11 @@ static int ath10k_sdio_hif_diag_read(struct ath10k *ar, u32 address, void *buf,
 	int ret;
 
 	/* set window register to start read cycle */
-	ret = ath10k_sdio_hif_set_addrwin_reg(ar,
-					      MBOX_WINDOW_READ_ADDR_ADDRESS,
-					      address);
-	if (ret)
+	ret = ath10k_sdio_write32(ar, MBOX_WINDOW_READ_ADDR_ADDRESS, address);
+	if (ret) {
+		ath10k_warn(ar, "failed to set mbox window read address: %d", ret);
 		return ret;
+	}
 
 	/* read the data */
 	ret = ath10k_sdio_read_write_sync(ar, MBOX_WINDOW_DATA_ADDRESS,
@@ -1595,9 +1574,13 @@ static int ath10k_sdio_hif_diag_write_mem(struct ath10k *ar, u32 address,
 	}
 
 	/* set window register, which starts the write cycle */
-	return ath10k_sdio_hif_set_addrwin_reg(ar,
-					       MBOX_WINDOW_WRITE_ADDR_ADDRESS,
-					       address);
+	ret = ath10k_sdio_write32(ar, MBOX_WINDOW_WRITE_ADDR_ADDRESS, address);
+	if (ret) {
+		ath10k_warn(ar, "failed to set mbox window write address: %d", ret);
+		return ret;
+	}
+
+	return 0;
 }
 
 /* HIF start/stop */
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index bc8d6e5342ea..d1e4f78ddaeb 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -168,9 +168,6 @@ struct ath10k_sdio_rx_data {
 #define HIF_RD_SYNC_BYTE_INC						\
 	(HIF_READ | HIF_BYTE_BASIS | HIF_INCREMENTAL_ADDRESS)
 
-#define HIF_RD_SYNC_BYTE_FIX						\
-	(HIF_READ | HIF_BYTE_BASIS | HIF_FIXED_ADDRESS)
-
 #define HIF_RD_SYNC_BLOCK_FIX						\
 	(HIF_READ | HIF_BLOCK_BASIS | HIF_FIXED_ADDRESS)
 
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 0/7] ath10k: remove ath10k_sdio_io()

[Erik Stromdahl]

On 2017-04-05 18:53, Kalle Valo wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>
>> I can't find patch 1/7...
>
> Odd, apparently it didn't get delivered to ath10k list but did get to
> linux-wireless. You can find the patch from linux-wireless patchwork:
>
> https://patchwork.kernel.org/patch/9663097/
>
> Also copying it below.
>
> Kalle
>

ok, thanks.

I will add the patches to my tree and try them out.

--
Erik

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 0/7] ath10k: remove ath10k_sdio_io()

[Erik Stromdahl]

I have made some basic testing (AP scanning) with this patchset and as far as I can
see they haven't broken anything.

I have not exercised all code (like error interrupt handling etc.) so those patches
might contain bugs (I noticed a FIXME comment in one of them).

I will review those patches and get back with more comments (if I find anything).

Erik

On 2017-04-05 09:06, Kalle Valo wrote:
> Using ath10k_sdio_io() and ath10k_sdio_read_write_sync() are just
> confusing. Refactor the code by following the plan outlined
> below.
>
> I'm planning to fold all these seven patches into Erik's patch
> adding sdio.c[1] so I didn't write any commit logs. I'm just
> submitting these separately to make it easier to review the
> changes.
>
> Kalle
>
> ath10k_sdio_read_write_sync():
> X HIF_RD_SYNC_BLOCK_FIX	-> ath10k_sdio_readsb()
> X HIF_RD_SYNC_BYTE_INC	-> ath10k_sdio_read(), ath10k_sdio_read32()
> X HIF_WR_SYNC_BYTE_FIX	-> ath10k_sdio_writesb32()
> X HIF_WR_SYNC_BYTE_INC	-> ath10k_sdio_write()
>
> ath10k_sdio_prep_async_req():
> X HIF_WRITE		-> HIF_WR_SYNC_BYTE_INC -> ath10k_sdio_write()
>
> ath10k_sdio_read/write32():
> sdio_readl()/sdio_writel() (those use endian macros)
>
> ath10k_sdio_writesb32():
> kmalloc()
> sdio_writesb(len)
> kfree()
>
> ath10k_sdio_readsb():
> len = round_down(len, ar_sdio->mbox_info.block_size);
> sdio_readsb(len)
>
> ath10k_sdio_read():
> sdio_memcpy_fromio()
>
> ath10k_sdio_write():
> sdio_memcpy_toio()
>
>
> [1] https://patchwork.kernel.org/patch/9626001/
>
> ---
>
> Kalle Valo (7):
>       ath10k: add ath10k_sdio_write32/read32()
>       ath10k: add ath10k_sdio_writesb()
>       ath10k: add ath10k_sdio_read()
>       ath10k: add ath10k_sdio_write()
>       ath10k: add ath10k_sdio_readsb()
>       ath10k: convert __ath10k_sdio_write_async() to use ath10k_sdio_write()
>       ath10k: remove unused sdio wrappers
>
>
>  drivers/net/wireless/ath/ath10k/sdio.c |  419 +++++++++++++++++---------------
>  drivers/net/wireless/ath/ath10k/sdio.h |   40 ---
>  2 files changed, 219 insertions(+), 240 deletions(-)
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k