linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Antipov <dmantipov@yandex.ru>
To: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: Kalle Valo <kvalo@kernel.org>,
	linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
	Dmitry Antipov <dmantipov@yandex.ru>
Subject: [PATCH] wifi: ath10k: simplify SDIO async handling
Date: Mon, 18 Sep 2023 17:37:15 +0300	[thread overview]
Message-ID: <20230918143718.78259-1-dmantipov@yandex.ru> (raw)

This is an initial attempt to fix TODO found in SDIO bus support
code. As original comment says, an idea is to store SDIO-specific
'struct ath10k_sdio_bus_request' data within skb control buffer
instead of managing (a relatively large, 1024-items for now) free
list of the aforementioned structures.

Compile tested only so TESTERS WANTED.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/ath/ath10k/sdio.c | 104 ++++++-------------------
 drivers/net/wireless/ath/ath10k/sdio.h |  14 +---
 2 files changed, 25 insertions(+), 93 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 56fbcfb80bf8..996f179206b9 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -1285,40 +1285,6 @@ static int ath10k_sdio_bmi_exchange_msg(struct ath10k *ar,
 
 /* sdio async handling functions */
 
-static struct ath10k_sdio_bus_request
-*ath10k_sdio_alloc_busreq(struct ath10k *ar)
-{
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct ath10k_sdio_bus_request *bus_req;
-
-	spin_lock_bh(&ar_sdio->lock);
-
-	if (list_empty(&ar_sdio->bus_req_freeq)) {
-		bus_req = NULL;
-		goto out;
-	}
-
-	bus_req = list_first_entry(&ar_sdio->bus_req_freeq,
-				   struct ath10k_sdio_bus_request, list);
-	list_del(&bus_req->list);
-
-out:
-	spin_unlock_bh(&ar_sdio->lock);
-	return bus_req;
-}
-
-static void ath10k_sdio_free_bus_req(struct ath10k *ar,
-				     struct ath10k_sdio_bus_request *bus_req)
-{
-	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-
-	memset(bus_req, 0, sizeof(*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 void __ath10k_sdio_write_async(struct ath10k *ar,
 				      struct ath10k_sdio_bus_request *req)
 {
@@ -1338,8 +1304,6 @@ static void __ath10k_sdio_write_async(struct ath10k *ar,
 	} else if (req->comp) {
 		complete(req->comp);
 	}
-
-	ath10k_sdio_free_bus_req(ar, req);
 }
 
 /* To improve throughput use workqueue to deliver packets to HTC layer,
@@ -1457,15 +1421,16 @@ static void ath10k_sdio_write_async_work(struct work_struct *work)
 	struct ath10k_sdio *ar_sdio = container_of(work, struct ath10k_sdio,
 						   wr_async_work);
 	struct ath10k *ar = ar_sdio->ar;
-	struct ath10k_sdio_bus_request *req, *tmp_req;
+	struct ath10k_sdio_bus_request *req;
 	struct ath10k_mbox_info *mbox_info = &ar_sdio->mbox_info;
+	struct sk_buff *skb;
 
 	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);
+	while ((skb = skb_dequeue(&ar_sdio->async_head))) {
 		spin_unlock_bh(&ar_sdio->wr_async_lock);
 
+		req = (struct ath10k_sdio_bus_request *)(skb->cb);
 		if (req->address >= mbox_info->htc_addr &&
 		    ar_sdio->mbox_state == SDIO_MBOX_SLEEP_STATE) {
 			ath10k_sdio_set_mbox_sleep(ar, false);
@@ -1483,23 +1448,15 @@ static void ath10k_sdio_write_async_work(struct work_struct *work)
 		ath10k_sdio_set_mbox_sleep(ar, true);
 }
 
-static int ath10k_sdio_prep_async_req(struct ath10k *ar, u32 addr,
-				      struct sk_buff *skb,
-				      struct completion *comp,
-				      bool htc_msg, enum ath10k_htc_ep_id eid)
+static void ath10k_sdio_prep_async_req(struct ath10k *ar, u32 addr,
+				       struct sk_buff *skb,
+				       struct completion *comp,
+				       bool htc_msg, enum ath10k_htc_ep_id eid)
 {
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
-	struct ath10k_sdio_bus_request *bus_req;
+	struct ath10k_sdio_bus_request *bus_req = (void *)skb->cb;
 
-	/* Allocate a bus request for the message and queue it on the
-	 * SDIO workqueue.
-	 */
-	bus_req = ath10k_sdio_alloc_busreq(ar);
-	if (!bus_req) {
-		ath10k_warn(ar,
-			    "unable to allocate bus request for async request\n");
-		return -ENOMEM;
-	}
+	BUILD_BUG_ON(sizeof(struct ath10k_sdio_bus_request) > sizeof(skb->cb));
 
 	bus_req->skb = skb;
 	bus_req->eid = eid;
@@ -1508,10 +1465,8 @@ static int ath10k_sdio_prep_async_req(struct ath10k *ar, u32 addr,
 	bus_req->comp = comp;
 
 	spin_lock_bh(&ar_sdio->wr_async_lock);
-	list_add_tail(&bus_req->list, &ar_sdio->wr_asyncq);
+	skb_queue_tail(&ar_sdio->async_head, skb);
 	spin_unlock_bh(&ar_sdio->wr_async_lock);
-
-	return 0;
 }
 
 /* IRQ handler */
@@ -1648,7 +1603,7 @@ static int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	enum ath10k_htc_ep_id eid;
 	struct sk_buff *skb;
-	int ret, i;
+	int i;
 
 	eid = pipe_id_to_eid(pipe_id);
 
@@ -1664,10 +1619,7 @@ static int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
 		/* Write TX data to the end of the mbox address space */
 		address = ar_sdio->mbox_addr[eid] + ar_sdio->mbox_size[eid] -
 			  skb->len;
-		ret = ath10k_sdio_prep_async_req(ar, address, skb,
-						 NULL, true, eid);
-		if (ret)
-			return ret;
+		ath10k_sdio_prep_async_req(ar, address, skb, NULL, true, eid);
 	}
 
 	queue_work(ar_sdio->workqueue, &ar_sdio->wr_async_work);
@@ -1934,10 +1886,8 @@ static void ath10k_sdio_irq_disable(struct ath10k *ar)
 	mutex_unlock(&irq_data->mtx);
 
 	init_completion(&irqs_disabled_comp);
-	ret = ath10k_sdio_prep_async_req(ar, MBOX_INT_STATUS_ENABLE_ADDRESS,
-					 skb, &irqs_disabled_comp, false, 0);
-	if (ret)
-		goto out;
+	ath10k_sdio_prep_async_req(ar, MBOX_INT_STATUS_ENABLE_ADDRESS,
+				   skb, &irqs_disabled_comp, false, 0);
 
 	queue_work(ar_sdio->workqueue, &ar_sdio->wr_async_work);
 
@@ -1957,14 +1907,13 @@ static void ath10k_sdio_irq_disable(struct ath10k *ar)
 
 	sdio_release_host(ar_sdio->func);
 
-out:
 	kfree_skb(skb);
 }
 
 static void ath10k_sdio_hif_stop(struct ath10k *ar)
 {
-	struct ath10k_sdio_bus_request *req, *tmp_req;
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
+	struct ath10k_sdio_bus_request *req;
 	struct sk_buff *skb;
 
 	ath10k_sdio_irq_disable(ar);
@@ -1979,18 +1928,16 @@ static void ath10k_sdio_hif_stop(struct ath10k *ar)
 	spin_lock_bh(&ar_sdio->wr_async_lock);
 
 	/* Free all bus requests that have not been handled */
-	list_for_each_entry_safe(req, tmp_req, &ar_sdio->wr_asyncq, list) {
+	while ((skb = skb_dequeue(&ar_sdio->async_head))) {
 		struct ath10k_htc_ep *ep;
 
-		list_del(&req->list);
-
+		req = (struct ath10k_sdio_bus_request *)(skb->cb);
 		if (req->htc_msg) {
 			ep = &ar->htc.endpoint[req->eid];
 			ath10k_htc_notify_tx_completion(ep, req->skb);
-		} else if (req->skb) {
-			kfree_skb(req->skb);
+		} else {
+			kfree_skb(skb);
 		}
-		ath10k_sdio_free_bus_req(ar, req);
 	}
 
 	spin_unlock_bh(&ar_sdio->wr_async_lock);
@@ -2514,7 +2461,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 	enum ath10k_hw_rev hw_rev;
 	u32 dev_id_base;
 	struct ath10k_bus_params bus_params = {};
-	int ret, i;
+	int ret;
 
 	/* Assumption: All SDIO based chipsets (so far) are QCA6174 based.
 	 * If there will be newer chipsets that does not use the hw reg
@@ -2574,13 +2521,9 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 	ar_sdio->is_disabled = true;
 	ar_sdio->ar = ar;
 
-	spin_lock_init(&ar_sdio->lock);
 	spin_lock_init(&ar_sdio->wr_async_lock);
 	mutex_init(&ar_sdio->irq_data.mtx);
 
-	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) {
@@ -2588,10 +2531,9 @@ static int ath10k_sdio_probe(struct sdio_func *func,
 		goto err_core_destroy;
 	}
 
-	for (i = 0; i < ATH10K_SDIO_BUS_REQUEST_MAX_NUM; i++)
-		ath10k_sdio_free_bus_req(ar, &ar_sdio->bus_req[i]);
-
 	skb_queue_head_init(&ar_sdio->rx_head);
+	skb_queue_head_init(&ar_sdio->async_head);
+
 	INIT_WORK(&ar_sdio->async_work_rx, ath10k_rx_indication_async_work);
 
 	dev_id_base = (id->device & 0x0F00);
diff --git a/drivers/net/wireless/ath/ath10k/sdio.h b/drivers/net/wireless/ath/ath10k/sdio.h
index b6ac927628b1..02d951581dc7 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.h
+++ b/drivers/net/wireless/ath/ath10k/sdio.h
@@ -29,7 +29,6 @@
 	(ATH10K_SDIO_MAX_BUFFER_SIZE - sizeof(struct ath10k_htc_hdr))
 
 #define ATH10K_HIF_MBOX_NUM_MAX                 4
-#define ATH10K_SDIO_BUS_REQUEST_MAX_NUM         1024
 
 #define ATH10K_SDIO_HIF_COMMUNICATION_TIMEOUT_HZ (100 * HZ)
 
@@ -104,10 +103,8 @@ enum sdio_mbox_state {
 #define ATH10K_CIS_READ_RETRY			10
 #define ATH10K_MIN_SLEEP_INACTIVITY_TIME_MS	50
 
-/* TODO: remove this and use skb->cb instead, much cleaner approach */
+/* Keep this small, see ath10k_sdio_prep_async_req() to check why. */
 struct ath10k_sdio_bus_request {
-	struct list_head list;
-
 	/* sdio address */
 	u32 address;
 
@@ -189,15 +186,8 @@ struct ath10k_sdio {
 	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;
-
 	struct sk_buff_head rx_head;
-
-	/* protects access to bus_req_freeq */
-	spinlock_t lock;
+	struct sk_buff_head async_head;
 
 	struct ath10k_sdio_rx_data rx_pkts[ATH10K_SDIO_MAX_RX_MSGS];
 	size_t n_rx_pkts;
-- 
2.41.0


             reply	other threads:[~2023-09-18 15:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-18 14:37 Dmitry Antipov [this message]
2023-09-19 14:42 ` [PATCH] wifi: ath10k: simplify SDIO async handling Jeff Johnson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230918143718.78259-1-dmantipov@yandex.ru \
    --to=dmantipov@yandex.ru \
    --cc=ath10k@lists.infradead.org \
    --cc=kvalo@kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=quic_jjohnson@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).