Message-ID: <20200115135338.14374-58-Jerome.Pouiller@silabs.com>
Date:   Wed, 15 Jan 2020 13:55:25 +0000
From:   Jérôme Pouiller <Jerome.Pouiller@...abs.com>
To:     "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jérôme Pouiller <Jerome.Pouiller@...abs.com>
Subject: [PATCH v2 57/65] staging: wfx: simplify handling of
 IEEE80211_TX_CTL_SEND_AFTER_DTIM

From: Jérôme Pouiller <jerome.pouiller@...abs.com>

When mac80211 ask for a frame to be sent after a DTIM, driver should:
  1. Update TIM with multicast bit set (using update_ie). This function
     can be called whenever.
  2. Keep buffered all frames marked "after dtim"
  3. When it receive a suspend_resume indication (see
     wfx_suspend_resume_mc()), send all the buffered frames. This
     indication is sent by the firmware 4ms before the dtim.
  4. If one of the frames returns status "REQUEUE", it means that the
     DTIM period was ended before to be able to send the frame.
  5. When all the buffered frames were sent or if DTIM period was ended,
     driver should update the TIM with multicast bit reset.

All the mess with the asynchronous works can be dropped.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
---
 drivers/staging/wfx/data_tx.c     | 17 +++---
 drivers/staging/wfx/hif_api_cmd.h |  3 +-
 drivers/staging/wfx/queue.c       | 98 +++++++++++++++----------------
 drivers/staging/wfx/queue.h       |  1 +
 drivers/staging/wfx/sta.c         | 78 ++----------------------
 drivers/staging/wfx/wfx.h         |  7 +--
 6 files changed, 66 insertions(+), 138 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 8710383f66e5..c32994553633 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -288,13 +288,6 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 	spin_lock_bh(&wvif->ps_state_lock);
 	if (ieee80211_is_auth(hdr->frame_control))
 		wvif->sta_asleep_mask &= mask;
-
-	if (tx_priv->link_id == WFX_LINK_ID_AFTER_DTIM &&
-	    !wvif->mcast_buffered) {
-		wvif->mcast_buffered = true;
-		if (wvif->sta_asleep_mask)
-			schedule_work(&wvif->mcast_start_work);
-	}
 	spin_unlock_bh(&wvif->ps_state_lock);
 
 	if (sta) {
@@ -479,6 +472,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	req->packet_id = queue_id << 16 |
 			 IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
 	req->data_flags.fc_offset = offset;
+	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
+		req->data_flags.after_dtim = 1;
 	req->queue_id.peer_sta_id = tx_priv->raw_link_id;
 	// Queue index are inverted between firmware and Linux
 	req->queue_id.queue_id = 3 - queue_id;
@@ -488,6 +483,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	// Auxiliary operations
 	wfx_tx_manage_pm(wvif, hdr, tx_priv, sta);
 	wfx_tx_queue_put(wvif->wdev, &wvif->wdev->tx_queue[queue_id], skb);
+	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
+		schedule_work(&wvif->update_tim_work);
 	wfx_bh_request_tx(wvif->wdev);
 	return 0;
 }
@@ -599,9 +596,11 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		else
 			tx_info->flags |= IEEE80211_TX_STAT_ACK;
 	} else if (arg->status == HIF_REQUEUE) {
-		/* "REQUEUE" means "implicit suspend" */
 		WARN(!arg->tx_result_flags.requeue, "incoherent status and result_flags");
-		wfx_suspend_resume_mc(wvif, STA_NOTIFY_SLEEP);
+		if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) {
+			wvif->after_dtim_tx_allowed = false; // DTIM period elapsed
+			schedule_work(&wvif->update_tim_work);
+		}
 		tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
 	} else {
 		if (wvif->bss_loss_state &&
diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index fc078d54bfbf..5554d6eddbf3 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -253,7 +253,8 @@ struct hif_queue {
 struct hif_data_flags {
 	u8    more:1;
 	u8    fc_offset:3;
-	u8    reserved:4;
+	u8    after_dtim:1;
+	u8    reserved:3;
 } __packed;
 
 struct hif_tx_flags {
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 42d64534c92c..ec11a63a2ff9 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -235,7 +235,6 @@ static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 			break;
 		}
 	}
-	WARN_ON(!skb);
 	if (skb) {
 		tx_priv = wfx_skb_tx_priv(skb);
 		tx_priv->xmit_timestamp = ktime_get();
@@ -473,23 +472,12 @@ static int wfx_get_prio_queue(struct wfx_vif *wvif,
 
 static int wfx_tx_queue_mask_get(struct wfx_vif *wvif,
 				     struct wfx_queue **queue_p,
-				     u32 *tx_allowed_mask_p,
-				     bool *more)
+				     u32 *tx_allowed_mask_p)
 {
 	int idx;
 	u32 tx_allowed_mask;
 	int total = 0;
 
-	/* Search for a queue with multicast frames buffered */
-	if (wvif->mcast_tx) {
-		tx_allowed_mask = BIT(WFX_LINK_ID_AFTER_DTIM);
-		idx = wfx_get_prio_queue(wvif, tx_allowed_mask, &total);
-		if (idx >= 0) {
-			*more = total > 1;
-			goto found;
-		}
-	}
-
 	/* Search for unicast traffic */
 	tx_allowed_mask = ~wvif->sta_asleep_mask;
 	tx_allowed_mask |= BIT(WFX_LINK_ID_UAPSD);
@@ -501,64 +489,83 @@ static int wfx_tx_queue_mask_get(struct wfx_vif *wvif,
 	if (idx < 0)
 		return -ENOENT;
 
-found:
 	*queue_p = &wvif->wdev->tx_queue[idx];
 	*tx_allowed_mask_p = tx_allowed_mask;
 	return 0;
 }
 
+struct hif_msg *wfx_tx_queues_get_after_dtim(struct wfx_vif *wvif)
+{
+	struct wfx_dev *wdev = wvif->wdev;
+	struct ieee80211_tx_info *tx_info;
+	struct hif_msg *hif;
+	struct sk_buff *skb;
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
+		skb_queue_walk(&wdev->tx_queue[i].queue, skb) {
+			tx_info = IEEE80211_SKB_CB(skb);
+			hif = (struct hif_msg *)skb->data;
+			if ((tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) &&
+			    (hif->interface == wvif->id))
+				return (struct hif_msg *)skb->data;
+		}
+	}
+	return NULL;
+}
+
 struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 {
 	struct sk_buff *skb;
 	struct hif_msg *hif = NULL;
-	struct hif_req_tx *req = NULL;
 	struct wfx_queue *queue = NULL;
 	struct wfx_queue *vif_queue = NULL;
 	u32 tx_allowed_mask = 0;
 	u32 vif_tx_allowed_mask = 0;
 	const struct wfx_tx_priv *tx_priv = NULL;
 	struct wfx_vif *wvif;
-	/* More is used only for broadcasts. */
-	bool more = false;
-	bool vif_more = false;
 	int not_found;
 	int burst;
+	int i;
+
+	if (atomic_read(&wdev->tx_lock))
+		return NULL;
+
+	wvif = NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
+		if (wvif->after_dtim_tx_allowed) {
+			for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
+				skb = wfx_tx_queue_get(wvif->wdev,
+						       &wdev->tx_queue[i],
+						       BIT(WFX_LINK_ID_AFTER_DTIM));
+				if (skb) {
+					hif = (struct hif_msg *)skb->data;
+					// Cannot happen since only one vif can
+					// be AP at time
+					WARN_ON(wvif->id != hif->interface);
+					return hif;
+				}
+			}
+			// No more multicast to sent
+			wvif->after_dtim_tx_allowed = false;
+			schedule_work(&wvif->update_tim_work);
+		}
+	}
 
 	for (;;) {
 		int ret = -ENOENT;
 		int queue_num;
-		struct ieee80211_hdr *hdr;
-
-		if (atomic_read(&wdev->tx_lock))
-			return NULL;
 
 		wvif = NULL;
 		while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
 			spin_lock_bh(&wvif->ps_state_lock);
 
 			not_found = wfx_tx_queue_mask_get(wvif, &vif_queue,
-							  &vif_tx_allowed_mask,
-							  &vif_more);
-
-			if (wvif->mcast_buffered && (not_found || !vif_more) &&
-					(wvif->mcast_tx ||
-					 !wvif->sta_asleep_mask)) {
-				wvif->mcast_buffered = false;
-				if (wvif->mcast_tx) {
-					wvif->mcast_tx = false;
-					schedule_work(&wvif->mcast_stop_work);
-				}
-			}
+							  &vif_tx_allowed_mask);
 
 			spin_unlock_bh(&wvif->ps_state_lock);
 
-			if (vif_more) {
-				more = true;
-				tx_allowed_mask = vif_tx_allowed_mask;
-				queue = vif_queue;
-				ret = 0;
-				break;
-			} else if (!not_found) {
+			if (!not_found) {
 				if (queue && queue != vif_queue)
 					dev_info(wdev->dev, "vifs disagree about queue priority\n");
 				tx_allowed_mask |= vif_tx_allowed_mask;
@@ -595,15 +602,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 		else
 			wdev->tx_burst_idx = -1;
 
-		/* more buffered multicast/broadcast frames
-		 *  ==> set MoreData flag in IEEE 802.11 header
-		 *  to inform PS STAs
-		 */
-		if (more) {
-			req = (struct hif_req_tx *) hif->body;
-			hdr = (struct ieee80211_hdr *) (req->frame + req->data_flags.fc_offset);
-			hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_MOREDATA);
-		}
 		return hif;
 	}
 }
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 813c2d09e034..096ae86135cc 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -47,6 +47,7 @@ void wfx_tx_queues_clear(struct wfx_dev *wdev);
 bool wfx_tx_queues_is_empty(struct wfx_dev *wdev);
 void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif);
 struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev);
+struct hif_msg *wfx_tx_queues_get_after_dtim(struct wfx_vif *wvif);
 
 void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
 		      struct sk_buff *skb);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index bdc15554958c..a9b58e4a9fa3 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -851,16 +851,12 @@ static void wfx_ps_notify_sta(struct wfx_vif *wvif,
 	switch (notify_cmd) {
 	case STA_NOTIFY_SLEEP:
 		if (!prev) {
-			if (wvif->mcast_buffered && !wvif->sta_asleep_mask)
-				schedule_work(&wvif->mcast_start_work);
 			wvif->sta_asleep_mask |= bit;
 		}
 		break;
 	case STA_NOTIFY_AWAKE:
 		if (prev) {
 			wvif->sta_asleep_mask &= ~bit;
-			if (!wvif->sta_asleep_mask)
-				schedule_work(&wvif->mcast_stop_work);
 			wfx_bh_request_tx(wvif->wdev);
 		}
 		break;
@@ -898,7 +894,7 @@ static int wfx_update_tim(struct wfx_vif *wvif)
 		tim_ptr[2] = 0;
 
 		/* Set/reset aid0 bit */
-		if (wvif->aid0_bit_set)
+		if (wfx_tx_queues_get_after_dtim(wvif))
 			tim_ptr[4] |= 1;
 		else
 			tim_ptr[4] &= ~1;
@@ -927,47 +923,12 @@ int wfx_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta, bool set)
 	return 0;
 }
 
-static void wfx_mcast_start_work(struct work_struct *work)
+void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd)
 {
-	struct wfx_vif *wvif =
-		container_of(work, struct wfx_vif, mcast_start_work);
-	struct ieee80211_bss_conf *conf = &wvif->vif->bss_conf;
-	long tmo = conf->dtim_period * TU_TO_JIFFIES(wvif->beacon_int + 20);
-
-	cancel_work_sync(&wvif->mcast_stop_work);
-	if (!wvif->aid0_bit_set) {
-		wfx_tx_lock_flush(wvif->wdev);
-		wvif->aid0_bit_set = true;
-		wfx_update_tim(wvif);
-		mod_timer(&wvif->mcast_timeout, jiffies + tmo);
-		wfx_tx_unlock(wvif->wdev);
-	}
-}
-
-static void wfx_mcast_stop_work(struct work_struct *work)
-{
-	struct wfx_vif *wvif = container_of(work, struct wfx_vif,
-					    mcast_stop_work);
-
-	if (wvif->aid0_bit_set) {
-		del_timer_sync(&wvif->mcast_timeout);
-		wfx_tx_lock_flush(wvif->wdev);
-		wvif->aid0_bit_set = false;
-		wfx_update_tim(wvif);
-		wfx_tx_unlock(wvif->wdev);
-	}
-}
-
-static void wfx_mcast_timeout(struct timer_list *t)
-{
-	struct wfx_vif *wvif = from_timer(wvif, t, mcast_timeout);
-
-	dev_warn(wvif->wdev->dev, "multicast delivery timeout\n");
-	spin_lock_bh(&wvif->ps_state_lock);
-	wvif->mcast_tx = wvif->aid0_bit_set && wvif->mcast_buffered;
-	if (wvif->mcast_tx)
-		wfx_bh_request_tx(wvif->wdev);
-	spin_unlock_bh(&wvif->ps_state_lock);
+	WARN(!wfx_tx_queues_get_after_dtim(wvif), "incorrect sequence");
+	WARN(wvif->after_dtim_tx_allowed, "incorrect sequence");
+	wvif->after_dtim_tx_allowed = true;
+	wfx_bh_request_tx(wvif->wdev);
 }
 
 int wfx_ampdu_action(struct ieee80211_hw *hw,
@@ -985,25 +946,6 @@ int wfx_ampdu_action(struct ieee80211_hw *hw,
 	return -ENOTSUPP;
 }
 
-void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd)
-{
-	bool cancel_tmo = false;
-
-	spin_lock_bh(&wvif->ps_state_lock);
-	if (notify_cmd == STA_NOTIFY_SLEEP)
-		wvif->mcast_tx = false;
-	else
-		wvif->mcast_tx = wvif->aid0_bit_set &&
-				 wvif->mcast_buffered;
-	if (wvif->mcast_tx) {
-		cancel_tmo = true;
-		wfx_bh_request_tx(wvif->wdev);
-	}
-	spin_unlock_bh(&wvif->ps_state_lock);
-	if (cancel_tmo)
-		del_timer_sync(&wvif->mcast_timeout);
-}
-
 int wfx_add_chanctx(struct ieee80211_hw *hw,
 		    struct ieee80211_chanctx_conf *conf)
 {
@@ -1090,10 +1032,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	spin_lock_init(&wvif->ps_state_lock);
 	INIT_WORK(&wvif->update_tim_work, wfx_update_tim_work);
 
-	INIT_WORK(&wvif->mcast_start_work, wfx_mcast_start_work);
-	INIT_WORK(&wvif->mcast_stop_work, wfx_mcast_stop_work);
-	timer_setup(&wvif->mcast_timeout, wfx_mcast_timeout, 0);
-
 	memset(&wvif->bss_params, 0, sizeof(wvif->bss_params));
 
 	mutex_init(&wvif->bss_loss_lock);
@@ -1156,9 +1094,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 		break;
 	case WFX_STATE_AP:
 		wvif->sta_asleep_mask = 0;
-		wvif->mcast_tx = false;
-		wvif->aid0_bit_set = false;
-		wvif->mcast_buffered = false;
 		/* reset.link_id = 0; */
 		hif_reset(wvif, false);
 		break;
@@ -1175,7 +1110,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 
 	wfx_cqm_bssloss_sm(wvif, 0, 0, 0);
 	cancel_work_sync(&wvif->unjoin_work);
-	del_timer_sync(&wvif->mcast_timeout);
 	wfx_free_event_queue(wvif);
 
 	wdev->vif[wvif->id] = NULL;
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 8491f050478d..5d61946e33e0 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -75,13 +75,8 @@ struct wfx_vif {
 
 	u32			link_id_map;
 
-	bool			aid0_bit_set;
-	bool			mcast_tx;
-	bool			mcast_buffered;
+	bool			after_dtim_tx_allowed;
 	struct wfx_grp_addr_table mcast_filter;
-	struct timer_list	mcast_timeout;
-	struct work_struct	mcast_start_work;
-	struct work_struct	mcast_stop_work;
 
 	s8			wep_default_key_id;
 	struct sk_buff		*wep_pending_skb;
-- 
2.25.0

Powered by blists - more mailing lists