[PATCH 00/32] staging: wfx: rework the Tx queue

[PATCH 00/32] staging: wfx: rework the Tx queue

[Jerome Pouiller]

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

This current implementation of the Tx queue is far more complex than
necessary and its behavior is dubious on some corner cases. This series
rework the Tx queue:
  - Simplify support for QoS (since device already do the job)
  - Simplify support for Power Saving stations (since mac80211 already
    do the job)
  - Improve support of Content After DTIM Beacon (CAB) when device is
    AP and some station use power save.
  - Improve robustness of flushing


Jérôme Pouiller (32):
  staging: wfx: add sanity checks to hif_join()
  staging: wfx: do not stop mac80211 queueing during tx_policy upload
  staging: wfx: take advantage of ieee80211_{stop/start}_queues
  staging: wfx: remove "burst" mechanism
  staging: wfx: uniformize queue_id retrieval
  staging: wfx: drop useless queue_id field
  staging: wfx: avoid useless wake_up
  staging: wfx: simplify hif_handle_tx_data()
  staging: wfx: simplify wfx_tx_queues_empty()
  staging: wfx: drop unused argument in wfx_get_prio_queue()
  staging: wfx: simplify wfx_tx_queue_mask_get()
  staging: wfx: drop useless sta_asleep_mask
  staging: wfx: drop argument tx_allowed_mask since it is constant now
  staging: wfx: do not use link_map_cache to track CAB
  staging: wfx: drop useless link_map_cache
  staging: wfx: do not rely anymore on link_id to choose packet in queue
  staging: wfx: drop unused link_id field
  staging: wfx: drop unused raw_link_id field
  staging: wfx: rename wfx_tx_get_raw_link_id()
  staging: wfx: replace wfx_tx_queues_get_after_dtim() by
    wfx_tx_queues_has_cab()
  staging: wfx: introduce a counter of pending frames
  staging: wfx: change the way to choose frame to send
  staging: wfx: drop now useless field edca_params
  staging: wfx: drop struct wfx_queue_stats
  staging: wfx: simplify usage of wfx_tx_queues_put()
  staging: wfx: improve interface between data_tx.c and queue.c
  staging: wfx: relocate wfx_skb_dtor() prior its callers
  staging: wfx: repair wfx_flush()
  staging: wfx: wfx_flush() did not ensure that frames are processed
  staging: wfx: fix potential deadlock in wfx_tx_flush()
  staging: wfx: fix case where AP stop with CAB traffic pending
  staging: wfx: remove hack about tx_rate policies

 drivers/staging/wfx/bh.c      |   4 +-
 drivers/staging/wfx/data_tx.c | 196 +++++--------
 drivers/staging/wfx/data_tx.h |   5 +-
 drivers/staging/wfx/hif_tx.c  |   2 +
 drivers/staging/wfx/main.c    |   2 -
 drivers/staging/wfx/queue.c   | 529 +++++++++++-----------------------
 drivers/staging/wfx/queue.h   |  36 +--
 drivers/staging/wfx/sta.c     |  87 ++----
 drivers/staging/wfx/sta.h     |   2 -
 drivers/staging/wfx/wfx.h     |   7 +-
 10 files changed, 281 insertions(+), 589 deletions(-)

-- 
2.25.1

[PATCH 01/32] staging: wfx: add sanity checks to hif_join()

[Jerome Pouiller]

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

Add a few check on start of hif_join().

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_tx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 77bca43aca42..445906035e9d 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -297,6 +297,8 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 	struct hif_req_join *body = wfx_alloc_hif(sizeof(*body), &hif);
 
 	WARN_ON(!conf->basic_rates);
+	WARN_ON(sizeof(body->ssid) < ssidlen);
+	WARN(!conf->ibss_joined && !ssidlen, "joining an unknown BSS");
 	body->infrastructure_bss_mode = !conf->ibss_joined;
 	body->short_preamble = conf->use_short_preamble;
 	if (channel && channel->flags & IEEE80211_CHAN_NO_IR)
-- 
2.25.1

Re: [PATCH 01/32] staging: wfx: add sanity checks to hif_join()

[Dan Carpenter]

On Wed, Apr 01, 2020 at 01:03:34PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> Add a few check on start of hif_join().
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/staging/wfx/hif_tx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> index 77bca43aca42..445906035e9d 100644
> --- a/drivers/staging/wfx/hif_tx.c
> +++ b/drivers/staging/wfx/hif_tx.c
> @@ -297,6 +297,8 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
>  	struct hif_req_join *body = wfx_alloc_hif(sizeof(*body), &hif);
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We've got an allocation here.  It's a mistake to put the allocation in
the declaration block because you're going to forget to check for
failure.

>  
>  	WARN_ON(!conf->basic_rates);
> +	WARN_ON(sizeof(body->ssid) < ssidlen);

Put the variable on the left.  WARN_ON(ssidlen > sizeof(body->ssid)).
I'm not a big fan of adding this sort of debug code, just audit the
callers to see if it's possible or not.

I have audited the caller for you, and I believe that this condition
*is possible* so we need to return -EINVAL in this situation to prevent
memory corruption.

	if (ssidlen > sizeof(body->ssid))
		return -EINVAL;

> +	WARN(!conf->ibss_joined && !ssidlen, "joining an unknown BSS");
>  	body->infrastructure_bss_mode = !conf->ibss_joined;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Potential NULL dererefence because of the unchecked allocation.

regards,
dan carpenter

Re: [PATCH 01/32] staging: wfx: add sanity checks to hif_join()

[Jérôme Pouiller]

On Thursday 2 April 2020 14:42:23 CEST Dan Carpenter wrote:
> On Wed, Apr 01, 2020 at 01:03:34PM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> >
> > Add a few check on start of hif_join().
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/staging/wfx/hif_tx.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
> > index 77bca43aca42..445906035e9d 100644
> > --- a/drivers/staging/wfx/hif_tx.c
> > +++ b/drivers/staging/wfx/hif_tx.c
> > @@ -297,6 +297,8 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
> >       struct hif_req_join *body = wfx_alloc_hif(sizeof(*body), &hif);
>                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> We've got an allocation here.  It's a mistake to put the allocation in
> the declaration block because you're going to forget to check for
> failure.

arf... this remark also applies to all functions of hif_tx.c. This
issue has already been reported. I will send a patch that solve that in
one batch.

> >       WARN_ON(!conf->basic_rates);
> > +     WARN_ON(sizeof(body->ssid) < ssidlen);
> 
> Put the variable on the left.  WARN_ON(ssidlen > sizeof(body->ssid)).
> I'm not a big fan of adding this sort of debug code, just audit the
> callers to see if it's possible or not.

My personal opinion is these checks does not replace the audit of the
callers. It mainly provides a kind of documentation for the reader
("not supported, please check the callers"). It is especially true when
it is an internal API and there is only one caller.

> I have audited the caller for you, and I believe that this condition
> *is possible* so we need to return -EINVAL in this situation to prevent
> memory corruption.
> 
>         if (ssidlen > sizeof(body->ssid))
>                 return -EINVAL;

In this case, I think the problem will also impact wfx_do_join() (the
only caller of hif_join()):

   514          u8 ssid[IEEE80211_MAX_SSID_LEN];
   [...]
   538          if (!conf->ibss_joined)
   539                  ssidie = ieee80211_bss_get_ie(bss, WLAN_EID_SSID);
   540          if (ssidie) {
   541                  ssidlen = ssidie[1];
   542                  memcpy(ssid, &ssidie[2], ssidie[1]);
   543          }
   [...]
   554          ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);

Does data returned by ieee80211_bss_get_ie() could be bigger than
IEEE80211_MAX_SSID_LEN? Not sure. I am going to add a check in
wfx_do_join(), just in case.


-- 
Jérôme Pouiller

[PATCH 02/32] staging: wfx: do not stop mac80211 queueing during tx_policy upload

[Jerome Pouiller]

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

When a new tx_policy has to be uploaded, it is necessary to avoid any
race between the frame and the policy. So, the driver stops the tx queue
during tx_policy upload. However, it is not necessary to stop mac80211
queuing.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 42183c70d4df..7a7c2518f6cf 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -244,9 +244,7 @@ void wfx_tx_policy_upload_work(struct work_struct *work)
 		container_of(work, struct wfx_vif, tx_policy_upload_work);
 
 	wfx_tx_policy_upload(wvif);
-
 	wfx_tx_unlock(wvif->wdev);
-	wfx_tx_queues_unlock(wvif->wdev);
 }
 
 void wfx_tx_policy_init(struct wfx_vif *wvif)
@@ -379,15 +377,9 @@ static u8 wfx_tx_get_rate_id(struct wfx_vif *wvif,
 		dev_warn(wvif->wdev->dev, "unable to get a valid Tx policy");
 
 	if (tx_policy_renew) {
-		/* FIXME: It's not so optimal to stop TX queues every now and
-		 * then.  Better to reimplement task scheduling with a counter.
-		 */
 		wfx_tx_lock(wvif->wdev);
-		wfx_tx_queues_lock(wvif->wdev);
-		if (!schedule_work(&wvif->tx_policy_upload_work)) {
-			wfx_tx_queues_unlock(wvif->wdev);
+		if (!schedule_work(&wvif->tx_policy_upload_work))
 			wfx_tx_unlock(wvif->wdev);
-		}
 	}
 	return rate_id;
 }
-- 
2.25.1

[PATCH 03/32] staging: wfx: take advantage of ieee80211_{stop/start}_queues

[Jerome Pouiller]

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

Instead of maintaining stop status for each queue, we can just maintain
a global status for all queues.

In add, wfx_tx_queues_{lock/unlock} are only used when no more
tx_policies are available. Therefore, the counter of recursive locks
("tx_locked_cnt") is useless.

So, wfx_tx_queues_{lock/unlock} can be replaced by
ieee80211_{stop/start}_queues.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 12 ++++--------
 drivers/staging/wfx/queue.c   | 29 -----------------------------
 drivers/staging/wfx/queue.h   |  3 ---
 3 files changed, 4 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 7a7c2518f6cf..889ebc2f5d83 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -189,10 +189,8 @@ static int wfx_tx_policy_get(struct wfx_vif *wvif,
 		idx = entry - cache->cache;
 	}
 	wfx_tx_policy_use(cache, &cache->cache[idx]);
-	if (list_empty(&cache->free)) {
-		/* Lock TX queues. */
-		wfx_tx_queues_lock(wvif->wdev);
-	}
+	if (list_empty(&cache->free))
+		ieee80211_stop_queues(wvif->wdev->hw);
 	spin_unlock_bh(&cache->lock);
 	return idx;
 }
@@ -207,10 +205,8 @@ static void wfx_tx_policy_put(struct wfx_vif *wvif, int idx)
 	spin_lock_bh(&cache->lock);
 	locked = list_empty(&cache->free);
 	usage = wfx_tx_policy_release(cache, &cache->cache[idx]);
-	if (locked && !usage) {
-		/* Unlock TX queues. */
-		wfx_tx_queues_unlock(wvif->wdev);
-	}
+	if (locked && !usage)
+		ieee80211_wake_queues(wvif->wdev->hw);
 	spin_unlock_bh(&cache->lock);
 }
 
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 39d9127ce4b9..e3aa1e346c70 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -55,35 +55,6 @@ void wfx_tx_lock_flush(struct wfx_dev *wdev)
 	wfx_tx_flush(wdev);
 }
 
-void wfx_tx_queues_lock(struct wfx_dev *wdev)
-{
-	int i;
-	struct wfx_queue *queue;
-
-	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
-		queue = &wdev->tx_queue[i];
-		spin_lock_bh(&queue->queue.lock);
-		if (queue->tx_locked_cnt++ == 0)
-			ieee80211_stop_queue(wdev->hw, queue->queue_id);
-		spin_unlock_bh(&queue->queue.lock);
-	}
-}
-
-void wfx_tx_queues_unlock(struct wfx_dev *wdev)
-{
-	int i;
-	struct wfx_queue *queue;
-
-	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
-		queue = &wdev->tx_queue[i];
-		spin_lock_bh(&queue->queue.lock);
-		WARN(!queue->tx_locked_cnt, "queue already unlocked");
-		if (--queue->tx_locked_cnt == 0)
-			ieee80211_wake_queue(wdev->hw, queue->queue_id);
-		spin_unlock_bh(&queue->queue.lock);
-	}
-}
-
 /* If successful, LOCKS the TX queue! */
 void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif)
 {
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 90bb060d1204..88ee2bf56d11 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -23,7 +23,6 @@ struct wfx_vif;
 
 struct wfx_queue {
 	struct sk_buff_head	queue;
-	int			tx_locked_cnt;
 	int			link_map_cache[WFX_LINK_ID_MAX];
 	u8			queue_id;
 };
@@ -41,8 +40,6 @@ void wfx_tx_lock_flush(struct wfx_dev *wdev);
 
 void wfx_tx_queues_init(struct wfx_dev *wdev);
 void wfx_tx_queues_deinit(struct wfx_dev *wdev);
-void wfx_tx_queues_lock(struct wfx_dev *wdev);
-void wfx_tx_queues_unlock(struct wfx_dev *wdev);
 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);
-- 
2.25.1

[PATCH 04/32] staging: wfx: remove "burst" mechanism

[Jerome Pouiller]

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

In the old days, the driver tried to reorder frames in order to send
frames from the same queue grouped to the firmware. However, the
firmware is able to do the job internally for a long time. There is no
reasons to keep this mechanism.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 23 -----------------------
 drivers/staging/wfx/sta.c   |  2 --
 drivers/staging/wfx/wfx.h   |  1 -
 3 files changed, 26 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index e3aa1e346c70..712ac783514b 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -363,8 +363,6 @@ static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
 static int wfx_get_prio_queue(struct wfx_vif *wvif,
 				 u32 tx_allowed_mask, int *total)
 {
-	static const int urgent = BIT(WFX_LINK_ID_AFTER_DTIM) |
-		BIT(WFX_LINK_ID_UAPSD);
 	const struct ieee80211_tx_queue_params *edca;
 	unsigned int score, best = -1;
 	int winner = -1;
@@ -389,14 +387,6 @@ static int wfx_get_prio_queue(struct wfx_vif *wvif,
 		}
 	}
 
-	/* override winner if bursting */
-	if (winner >= 0 && wvif->wdev->tx_burst_idx >= 0 &&
-	    winner != wvif->wdev->tx_burst_idx &&
-	    !wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[winner],
-					 tx_allowed_mask & urgent) &&
-	    wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[wvif->wdev->tx_burst_idx], tx_allowed_mask))
-		winner = wvif->wdev->tx_burst_idx;
-
 	return winner;
 }
 
@@ -454,7 +444,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 	u32 vif_tx_allowed_mask = 0;
 	struct wfx_vif *wvif;
 	int not_found;
-	int burst;
 	int i;
 
 	if (atomic_read(&wdev->tx_lock))
@@ -518,18 +507,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 		if (hif_handle_tx_data(wvif, skb, queue))
 			continue;  /* Handled by WSM */
 
-		/* allow bursting if txop is set */
-		if (wvif->edca_params[queue_num].txop)
-			burst = wfx_tx_queue_get_num_queued(queue, tx_allowed_mask) + 1;
-		else
-			burst = 1;
-
-		/* store index of bursting queue */
-		if (burst > 1)
-			wdev->tx_burst_idx = queue_num;
-		else
-			wdev->tx_burst_idx = -1;
-
 		return hif;
 	}
 }
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 9d430346a58b..a275330f5518 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -531,7 +531,6 @@ static void wfx_do_join(struct wfx_vif *wvif)
 
 	wfx_set_mfp(wvif, bss);
 
-	wvif->wdev->tx_burst_idx = -1;
 	ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);
 	if (ret) {
 		ieee80211_connection_loss(wvif->vif);
@@ -624,7 +623,6 @@ static int wfx_start_ap(struct wfx_vif *wvif)
 	int ret;
 
 	wvif->beacon_int = wvif->vif->bss_conf.beacon_int;
-	wvif->wdev->tx_burst_idx = -1;
 	ret = hif_start(wvif, &wvif->vif->bss_conf, wvif->channel);
 	if (ret)
 		return ret;
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 8b85bb1abb9c..116f456a5da2 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -51,7 +51,6 @@ struct wfx_dev {
 	struct wfx_hif_cmd	hif_cmd;
 	struct wfx_queue	tx_queue[4];
 	struct wfx_queue_stats	tx_queue_stats;
-	int			tx_burst_idx;
 	atomic_t		tx_lock;
 
 	atomic_t		packet_id;
-- 
2.25.1

Re: [PATCH 04/32] staging: wfx: remove "burst" mechanism

[Dan Carpenter]

On Wed, Apr 01, 2020 at 01:03:37PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> In the old days, the driver tried to reorder frames in order to send
> frames from the same queue grouped to the firmware. However, the
> firmware is able to do the job internally for a long time. There is no
> reasons to keep this mechanism.
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/staging/wfx/queue.c | 23 -----------------------
>  drivers/staging/wfx/sta.c   |  2 --
>  drivers/staging/wfx/wfx.h   |  1 -
>  3 files changed, 26 deletions(-)
> 
> diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
> index e3aa1e346c70..712ac783514b 100644
> --- a/drivers/staging/wfx/queue.c
> +++ b/drivers/staging/wfx/queue.c
> @@ -363,8 +363,6 @@ static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
>  static int wfx_get_prio_queue(struct wfx_vif *wvif,
>  				 u32 tx_allowed_mask, int *total)
>  {
> -	static const int urgent = BIT(WFX_LINK_ID_AFTER_DTIM) |
> -		BIT(WFX_LINK_ID_UAPSD);
>  	const struct ieee80211_tx_queue_params *edca;
>  	unsigned int score, best = -1;
                            ^^^^^^^^^
Not related to this this patch but this confused me initially.  UINT_MAX
would be more readable.

The other unrelated question I had about this function was:

   402          /* search for a winner using edca params */
   403          for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
                                ^^^^^^^^^^^^^^^^^
IEEE80211_NUM_ACS is 4.

   404                  int queued;
   405  
   406                  edca = &wvif->edca_params[i];
   407                  queued = wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i],
   408                                  tx_allowed_mask);
   409                  if (!queued)
   410                          continue;
   411                  *total += queued;
   412                  score = ((edca->aifs + edca->cw_min) << 16) +
   413                          ((edca->cw_max - edca->cw_min) *
   414                           (get_random_int() & 0xFFFF));
   415                  if (score < best && (winner < 0 || i != 3)) {
                                                           ^^^^^^

Why do we not want winner to be 3?  It's unrelated to the patch but
there should be a comment next to that code probably.

   416                          best = score;
   417                          winner = i;
   418                  }
   419          }

regards,
dan carpenter

Re: [PATCH 04/32] staging: wfx: remove "burst" mechanism

[Jérôme Pouiller]

On Thursday 2 April 2020 15:05:26 CEST Dan Carpenter wrote:
[...]
>                             ^^^^^^^^^
> Not related to this this patch but this confused me initially.  UINT_MAX
> would be more readable.
> 
> The other unrelated question I had about this function was:
> 
>    402          /* search for a winner using edca params */
>    403          for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
>                                 ^^^^^^^^^^^^^^^^^
> IEEE80211_NUM_ACS is 4.
> 
>    404                  int queued;
>    405
>    406                  edca = &wvif->edca_params[i];
>    407                  queued = wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i],
>    408                                  tx_allowed_mask);
>    409                  if (!queued)
>    410                          continue;
>    411                  *total += queued;
>    412                  score = ((edca->aifs + edca->cw_min) << 16) +
>    413                          ((edca->cw_max - edca->cw_min) *
>    414                           (get_random_int() & 0xFFFF));
>    415                  if (score < best && (winner < 0 || i != 3)) {
>                                                            ^^^^^^
> 
> Why do we not want winner to be 3?  It's unrelated to the patch but
> there should be a comment next to that code probably.
> 
>    416                          best = score;
>    417                          winner = i;
>    418                  }
>    419          }

Indeed. In add, this code is useless. That's why I drop this code in
patch 22/32.

-- 
Jérôme Pouiller

[PATCH 05/32] staging: wfx: uniformize queue_id retrieval

[Jerome Pouiller]

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

tx_info->hw_queue contains "vif.hw_queue[skb_get_queue_mapping(skb)]".
For now, it is equivalent of "skb_get_queue_mapping(skb)". However, it
is not the same semantic. In wfx_tx_inner(), we want to get the mac80211
queue index, not the hardware queue index.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 889ebc2f5d83..8e4c3e1aadfd 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -417,7 +417,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_key_conf *hw_key = tx_info->control.hw_key;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	int queue_id = tx_info->hw_queue;
+	int queue_id = skb_get_queue_mapping(skb);
 	size_t offset = (size_t)skb->data & 3;
 	int wmsg_len = sizeof(struct hif_msg) +
 			sizeof(struct hif_req_tx) + offset;
-- 
2.25.1

[PATCH 06/32] staging: wfx: drop useless queue_id field

[Jerome Pouiller]

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

The field queue_id is no more used.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 4 +---
 drivers/staging/wfx/queue.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 712ac783514b..1df3b6f28c67 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -134,10 +134,8 @@ void wfx_tx_queues_init(struct wfx_dev *wdev)
 	skb_queue_head_init(&wdev->tx_queue_stats.pending);
 	init_waitqueue_head(&wdev->tx_queue_stats.wait_link_id_empty);
 
-	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
-		wdev->tx_queue[i].queue_id = i;
+	for (i = 0; i < IEEE80211_NUM_ACS; ++i)
 		skb_queue_head_init(&wdev->tx_queue[i].queue);
-	}
 }
 
 void wfx_tx_queues_deinit(struct wfx_dev *wdev)
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 88ee2bf56d11..2284fa64b625 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -24,7 +24,6 @@ struct wfx_vif;
 struct wfx_queue {
 	struct sk_buff_head	queue;
 	int			link_map_cache[WFX_LINK_ID_MAX];
-	u8			queue_id;
 };
 
 struct wfx_queue_stats {
-- 
2.25.1

[PATCH 07/32] staging: wfx: avoid useless wake_up

[Jerome Pouiller]

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

__wfx_flush() wait for all queues to be empty. In current code,
wait_link_id_empty is wake up each time there is no more data for a
station. We can simplify the processing and avoid some wake-up by
raising this event only when the queue is empty.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 1df3b6f28c67..2553f77522d9 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -190,7 +190,6 @@ static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 	struct sk_buff *item;
 	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 	struct wfx_tx_priv *tx_priv;
-	bool wakeup_stats = false;
 
 	spin_lock_bh(&queue->queue.lock);
 	skb_queue_walk(&queue->queue, item) {
@@ -208,12 +207,11 @@ static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 
 		spin_lock_nested(&stats->pending.lock, 1);
 		__skb_queue_tail(&stats->pending, skb);
-		if (!--stats->link_map_cache[tx_priv->link_id])
-			wakeup_stats = true;
+		--stats->link_map_cache[tx_priv->link_id];
 		spin_unlock(&stats->pending.lock);
 	}
 	spin_unlock_bh(&queue->queue.lock);
-	if (wakeup_stats)
+	if (skb_queue_empty(&queue->queue))
 		wake_up(&stats->wait_link_id_empty);
 	return skb;
 }
-- 
2.25.1

[PATCH 08/32] staging: wfx: simplify hif_handle_tx_data()

[Jerome Pouiller]

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

The last argument of hif_handle_tx_data() was now unused. In add,
hif_handle_tx_data() has nothing to do with HIF layer and should be
renamed. Finally, it not convenient to pass a wfx_vif as parameter. It
is easier to let hif_handle_tx_data() find the interface itself.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 2553f77522d9..8647731e02c0 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -319,13 +319,17 @@ bool wfx_tx_queues_is_empty(struct wfx_dev *wdev)
 	return ret;
 }
 
-static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
-			       struct wfx_queue *queue)
+static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
 {
 	struct hif_req_tx *req = wfx_skb_txreq(skb);
 	struct ieee80211_key_conf *hw_key = wfx_skb_tx_priv(skb)->hw_key;
 	struct ieee80211_hdr *frame =
 		(struct ieee80211_hdr *)(req->frame + req->data_flags.fc_offset);
+	struct wfx_vif *wvif =
+		wdev_to_wvif(wdev, ((struct hif_msg *)skb->data)->interface);
+
+	if (!wvif)
+		return false;
 
 	// FIXME: mac80211 is smart enough to handle BSS loss. Driver should not
 	// try to do anything about that.
@@ -344,12 +348,12 @@ static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
 	    hw_key && hw_key->keyidx != wvif->wep_default_key_id &&
 	    (hw_key->cipher == WLAN_CIPHER_SUITE_WEP40 ||
 	     hw_key->cipher == WLAN_CIPHER_SUITE_WEP104)) {
-		wfx_tx_lock(wvif->wdev);
+		wfx_tx_lock(wdev);
 		WARN_ON(wvif->wep_pending_skb);
 		wvif->wep_default_key_id = hw_key->keyidx;
 		wvif->wep_pending_skb = skb;
 		if (!schedule_work(&wvif->wep_key_work))
-			wfx_tx_unlock(wvif->wdev);
+			wfx_tx_unlock(wdev);
 		return true;
 	} else {
 		return false;
@@ -496,13 +500,10 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 		skb = wfx_tx_queue_get(wdev, queue, tx_allowed_mask);
 		if (!skb)
 			continue;
-		hif = (struct hif_msg *)skb->data;
-		wvif = wdev_to_wvif(wdev, hif->interface);
-		WARN_ON(!wvif);
 
-		if (hif_handle_tx_data(wvif, skb, queue))
+		if (wfx_handle_tx_data(wdev, skb))
 			continue;  /* Handled by WSM */
 
-		return hif;
+		return (struct hif_msg *)skb->data;
 	}
 }
-- 
2.25.1

Re: [PATCH 08/32] staging: wfx: simplify hif_handle_tx_data()

[Dan Carpenter]

On Wed, Apr 01, 2020 at 01:03:41PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> The last argument of hif_handle_tx_data() was now unused. In add,
> hif_handle_tx_data() has nothing to do with HIF layer and should be
> renamed. Finally, it not convenient to pass a wfx_vif as parameter. It
> is easier to let hif_handle_tx_data() find the interface itself.
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/staging/wfx/queue.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
> index 2553f77522d9..8647731e02c0 100644
> --- a/drivers/staging/wfx/queue.c
> +++ b/drivers/staging/wfx/queue.c
> @@ -319,13 +319,17 @@ bool wfx_tx_queues_is_empty(struct wfx_dev *wdev)
>  	return ret;
>  }
>  
> -static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
> -			       struct wfx_queue *queue)
> +static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
>  {
>  	struct hif_req_tx *req = wfx_skb_txreq(skb);
>  	struct ieee80211_key_conf *hw_key = wfx_skb_tx_priv(skb)->hw_key;
>  	struct ieee80211_hdr *frame =
>  		(struct ieee80211_hdr *)(req->frame + req->data_flags.fc_offset);
> +	struct wfx_vif *wvif =
> +		wdev_to_wvif(wdev, ((struct hif_msg *)skb->data)->interface);
                                                      ^^^^^^^^^
This is on the TX side so it's probably okay, but one problem I have
noticed is that we do this on the RX side as well with checking that

	if (skb->len < sizeof(struct hif_msg))
		return -EINVAL;

So we could be reading beyond the end of the skb.  If we got really
unlucky it could lead to an Oops.

regards,
dan carpenter

Re: [PATCH 08/32] staging: wfx: simplify hif_handle_tx_data()

[Jérôme Pouiller]

On Thursday 2 April 2020 15:13:39 CEST Dan Carpenter wrote:
> On Wed, Apr 01, 2020 at 01:03:41PM +0200, Jerome Pouiller wrote:
[...]
> This is on the TX side so it's probably okay, but one problem I have
> noticed is that we do this on the RX side as well with checking that
> 
>         if (skb->len < sizeof(struct hif_msg))
>                 return -EINVAL;
> 
> So we could be reading beyond the end of the skb.  If we got really
> unlucky it could lead to an Oops.
> 
> regards,
> dan carpenter
> 
> 
Hello Dan,

The function rx_helper() in bh.c already do some sanity checks received data:

    60          WARN(read_len < 4, "corrupted read");
    [...]
    92          } else {
    93                  computed_len = round_up(hif->len, 2);
    94          }
    95          if (computed_len != read_len) {
    96                  dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
    97                          computed_len, read_len);
    98                  print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET, 16, 1,
    99                                 hif, read_len, true);
   100                  goto err;
   101          }


However, I can improve this code:
   - "4" should be replaced by "sizeof(struct hif_msg)" for readability 
   - hif->len is tested through computed_len, but I am not sure to be able
     to prove that it covers all cases
   - rx_helper() should recover the error if read_len < 4

I add that on my TODO list.

-- 
Jérôme Pouiller

[PATCH 09/32] staging: wfx: simplify wfx_tx_queues_empty()

[Jerome Pouiller]

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

Thanks to skb_queue_empty_lockless(), it is not necessary to acquire the
spin_lock before to check if the queue is empty.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 16 +++++-----------
 drivers/staging/wfx/queue.h |  2 +-
 drivers/staging/wfx/sta.c   |  4 ++--
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 8647731e02c0..09f823929fb6 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -303,20 +303,14 @@ unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 	return ktime_us_delta(now, tx_priv->xmit_timestamp);
 }
 
-bool wfx_tx_queues_is_empty(struct wfx_dev *wdev)
+bool wfx_tx_queues_empty(struct wfx_dev *wdev)
 {
 	int i;
-	struct sk_buff_head *queue;
-	bool ret = true;
 
-	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
-		queue = &wdev->tx_queue[i].queue;
-		spin_lock_bh(&queue->lock);
-		if (!skb_queue_empty(queue))
-			ret = false;
-		spin_unlock_bh(&queue->lock);
-	}
-	return ret;
+	for (i = 0; i < IEEE80211_NUM_ACS; i++)
+		if (!skb_queue_empty_lockless(&wdev->tx_queue[i].queue))
+			return false;
+	return true;
 }
 
 static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 2284fa64b625..5a5aa38dbb2f 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -40,7 +40,7 @@ void wfx_tx_lock_flush(struct wfx_dev *wdev);
 void wfx_tx_queues_init(struct wfx_dev *wdev);
 void wfx_tx_queues_deinit(struct wfx_dev *wdev);
 void wfx_tx_queues_clear(struct wfx_dev *wdev);
-bool wfx_tx_queues_is_empty(struct wfx_dev *wdev);
+bool wfx_tx_queues_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);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index a275330f5518..be493b5f2b5d 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -325,11 +325,11 @@ static int __wfx_flush(struct wfx_dev *wdev, bool drop)
 		if (drop)
 			wfx_tx_queues_clear(wdev);
 		if (wait_event_timeout(wdev->tx_queue_stats.wait_link_id_empty,
-				       wfx_tx_queues_is_empty(wdev),
+				       wfx_tx_queues_empty(wdev),
 				       2 * HZ) <= 0)
 			return -ETIMEDOUT;
 		wfx_tx_flush(wdev);
-		if (wfx_tx_queues_is_empty(wdev))
+		if (wfx_tx_queues_empty(wdev))
 			return 0;
 		dev_warn(wdev->dev, "frames queued while flushing tx queues");
 	}
-- 
2.25.1

[PATCH 10/32] staging: wfx: drop unused argument in wfx_get_prio_queue()

[Jerome Pouiller]

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

The argument "total" is not used anymore since commit a3c529a83589
("staging: wfx: simplify handling of IEEE80211_TX_CTL_SEND_AFTER_DTIM").

Fixes: a3c529a83589 ("staging: wfx: simplify handling of IEEE80211_TX_CTL_SEND_AFTER_DTIM")
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 09f823929fb6..bbab6b192b0c 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -354,8 +354,7 @@ static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
 	}
 }
 
-static int wfx_get_prio_queue(struct wfx_vif *wvif,
-				 u32 tx_allowed_mask, int *total)
+static int wfx_get_prio_queue(struct wfx_vif *wvif, u32 tx_allowed_mask)
 {
 	const struct ieee80211_tx_queue_params *edca;
 	unsigned int score, best = -1;
@@ -371,7 +370,6 @@ static int wfx_get_prio_queue(struct wfx_vif *wvif,
 				tx_allowed_mask);
 		if (!queued)
 			continue;
-		*total += queued;
 		score = ((edca->aifs + edca->cw_min) << 16) +
 			((edca->cw_max - edca->cw_min) *
 			 (get_random_int() & 0xFFFF));
@@ -390,7 +388,6 @@ static int wfx_tx_queue_mask_get(struct wfx_vif *wvif,
 {
 	int idx;
 	u32 tx_allowed_mask;
-	int total = 0;
 
 	/* Search for unicast traffic */
 	tx_allowed_mask = ~wvif->sta_asleep_mask;
@@ -399,7 +396,7 @@ static int wfx_tx_queue_mask_get(struct wfx_vif *wvif,
 		tx_allowed_mask &= ~BIT(WFX_LINK_ID_AFTER_DTIM);
 	else
 		tx_allowed_mask |= BIT(WFX_LINK_ID_AFTER_DTIM);
-	idx = wfx_get_prio_queue(wvif, tx_allowed_mask, &total);
+	idx = wfx_get_prio_queue(wvif, tx_allowed_mask);
 	if (idx < 0)
 		return -ENOENT;
 
-- 
2.25.1

[PATCH 11/32] staging: wfx: simplify wfx_tx_queue_mask_get()

[Jerome Pouiller]

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

It is far simpler to return a pointer instead of an error. Thus, it is
no more necessary to pass a pointer reference as argument.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index bbab6b192b0c..c602496580f7 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -382,9 +382,8 @@ static int wfx_get_prio_queue(struct wfx_vif *wvif, u32 tx_allowed_mask)
 	return winner;
 }
 
-static int wfx_tx_queue_mask_get(struct wfx_vif *wvif,
-				     struct wfx_queue **queue_p,
-				     u32 *tx_allowed_mask_p)
+static struct wfx_queue *wfx_tx_queue_mask_get(struct wfx_vif *wvif,
+					       u32 *tx_allowed_mask_p)
 {
 	int idx;
 	u32 tx_allowed_mask;
@@ -398,11 +397,10 @@ static int wfx_tx_queue_mask_get(struct wfx_vif *wvif,
 		tx_allowed_mask |= BIT(WFX_LINK_ID_AFTER_DTIM);
 	idx = wfx_get_prio_queue(wvif, tx_allowed_mask);
 	if (idx < 0)
-		return -ENOENT;
+		return NULL;
 
-	*queue_p = &wvif->wdev->tx_queue[idx];
 	*tx_allowed_mask_p = tx_allowed_mask;
-	return 0;
+	return &wvif->wdev->tx_queue[idx];
 }
 
 struct hif_msg *wfx_tx_queues_get_after_dtim(struct wfx_vif *wvif)
@@ -434,7 +432,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 	u32 tx_allowed_mask = 0;
 	u32 vif_tx_allowed_mask = 0;
 	struct wfx_vif *wvif;
-	int not_found;
 	int i;
 
 	if (atomic_read(&wdev->tx_lock))
@@ -469,12 +466,12 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 		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_queue = wfx_tx_queue_mask_get(wvif,
 							  &vif_tx_allowed_mask);
 
 			spin_unlock_bh(&wvif->ps_state_lock);
 
-			if (!not_found) {
+			if (vif_queue) {
 				if (queue && queue != vif_queue)
 					dev_info(wdev->dev, "vifs disagree about queue priority\n");
 				tx_allowed_mask |= vif_tx_allowed_mask;
-- 
2.25.1

[PATCH 12/32] staging: wfx: drop useless sta_asleep_mask

[Jerome Pouiller]

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

Currently, the driver tracks power save state of the stations with the
variable sta_asleep_mask. Then, it takes care to not sent data to asleep
stations.

However, this work is already done by mac80211. Normally, there are no
frames for asleep stations in our queues. So, driver do not have to
filter frames in its queues (apart the frames marked "AFTER_DTIM").

Notice that there is a risk of race between state of the station and
data send to the firmware. However, this risk is limited since the
number of frame in queues are small. In add, this race also exists with
the current code. Anyway, the firmware is able to detect the problem and
driver will receive a 'REQUEUE' status (translated in
TX_STAT_TX_FILTERED for mac80211).

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c |  6 ------
 drivers/staging/wfx/main.c    |  1 -
 drivers/staging/wfx/queue.c   | 14 ++------------
 drivers/staging/wfx/sta.c     | 29 -----------------------------
 drivers/staging/wfx/wfx.h     |  2 --
 5 files changed, 2 insertions(+), 50 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 8e4c3e1aadfd..a53e6d15031b 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -275,15 +275,9 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 			     struct wfx_tx_priv *tx_priv,
 			     struct ieee80211_sta *sta)
 {
-	u32 mask = ~BIT(tx_priv->raw_link_id);
 	struct wfx_sta_priv *sta_priv;
 	int tid = ieee80211_get_tid(hdr);
 
-	spin_lock_bh(&wvif->ps_state_lock);
-	if (ieee80211_is_auth(hdr->frame_control))
-		wvif->sta_asleep_mask &= mask;
-	spin_unlock_bh(&wvif->ps_state_lock);
-
 	if (sta) {
 		sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
 		spin_lock_bh(&sta_priv->lock);
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 3c4c240229ad..5e1a7a932b53 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -138,7 +138,6 @@ static const struct ieee80211_ops wfx_ops = {
 	.cancel_hw_scan		= wfx_cancel_hw_scan,
 	.sta_add		= wfx_sta_add,
 	.sta_remove		= wfx_sta_remove,
-	.sta_notify		= wfx_sta_notify,
 	.set_tim		= wfx_set_tim,
 	.set_key		= wfx_set_key,
 	.set_rts_threshold	= wfx_set_rts_threshold,
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index c602496580f7..e66debd60e3f 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -388,13 +388,8 @@ static struct wfx_queue *wfx_tx_queue_mask_get(struct wfx_vif *wvif,
 	int idx;
 	u32 tx_allowed_mask;
 
-	/* Search for unicast traffic */
-	tx_allowed_mask = ~wvif->sta_asleep_mask;
-	tx_allowed_mask |= BIT(WFX_LINK_ID_UAPSD);
-	if (wvif->sta_asleep_mask)
-		tx_allowed_mask &= ~BIT(WFX_LINK_ID_AFTER_DTIM);
-	else
-		tx_allowed_mask |= BIT(WFX_LINK_ID_AFTER_DTIM);
+	tx_allowed_mask = BIT(WFX_LINK_ID_MAX) - 1;
+	tx_allowed_mask &= ~BIT(WFX_LINK_ID_AFTER_DTIM);
 	idx = wfx_get_prio_queue(wvif, tx_allowed_mask);
 	if (idx < 0)
 		return NULL;
@@ -464,13 +459,8 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 
 		wvif = NULL;
 		while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
-			spin_lock_bh(&wvif->ps_state_lock);
-
 			vif_queue = wfx_tx_queue_mask_get(wvif,
 							  &vif_tx_allowed_mask);
-
-			spin_unlock_bh(&wvif->ps_state_lock);
-
 			if (vif_queue) {
 				if (queue && queue != vif_queue)
 					dev_info(wdev->dev, "vifs disagree about queue priority\n");
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index be493b5f2b5d..5c5b52dc7bdd 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -590,11 +590,6 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	WARN_ON(sta_priv->link_id >= WFX_MAX_STA_IN_AP_MODE);
 	hif_map_link(wvif, sta->addr, 0, sta_priv->link_id);
 
-	spin_lock_bh(&wvif->ps_state_lock);
-	if ((sta->uapsd_queues & IEEE80211_WMM_IE_STA_QOSINFO_AC_MASK) ==
-					IEEE80211_WMM_IE_STA_QOSINFO_AC_MASK)
-		wvif->sta_asleep_mask |= BIT(sta_priv->link_id);
-	spin_unlock_bh(&wvif->ps_state_lock);
 	return 0;
 }
 
@@ -841,28 +836,6 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw,
 		wfx_do_join(wvif);
 }
 
-static void wfx_ps_notify_sta(struct wfx_vif *wvif,
-			      enum sta_notify_cmd notify_cmd, int link_id)
-{
-	spin_lock_bh(&wvif->ps_state_lock);
-	if (notify_cmd == STA_NOTIFY_SLEEP)
-		wvif->sta_asleep_mask |= BIT(link_id);
-	else // notify_cmd == STA_NOTIFY_AWAKE
-		wvif->sta_asleep_mask &= ~BIT(link_id);
-	spin_unlock_bh(&wvif->ps_state_lock);
-	if (notify_cmd == STA_NOTIFY_AWAKE)
-		wfx_bh_request_tx(wvif->wdev);
-}
-
-void wfx_sta_notify(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
-		    enum sta_notify_cmd notify_cmd, struct ieee80211_sta *sta)
-{
-	struct wfx_vif *wvif = (struct wfx_vif *) vif->drv_priv;
-	struct wfx_sta_priv *sta_priv = (struct wfx_sta_priv *) &sta->drv_priv;
-
-	wfx_ps_notify_sta(wvif, notify_cmd, sta_priv->link_id);
-}
-
 static int wfx_update_tim(struct wfx_vif *wvif)
 {
 	struct sk_buff *skb;
@@ -1019,7 +992,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	wvif->wdev = wdev;
 
 	wvif->link_id_map = 1; // link-id 0 is reserved for multicast
-	spin_lock_init(&wvif->ps_state_lock);
 	INIT_WORK(&wvif->update_tim_work, wfx_update_tim_work);
 
 	memset(&wvif->bss_params, 0, sizeof(wvif->bss_params));
@@ -1083,7 +1055,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 			wfx_tx_unlock(wdev);
 		break;
 	case WFX_STATE_AP:
-		wvif->sta_asleep_mask = 0;
 		/* reset.link_id = 0; */
 		hif_reset(wvif, false);
 		break;
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 116f456a5da2..61899cd7942b 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -85,8 +85,6 @@ struct wfx_vif {
 	struct tx_policy_cache	tx_policy_cache;
 	struct work_struct	tx_policy_upload_work;
 
-	u32			sta_asleep_mask;
-	spinlock_t		ps_state_lock;
 	struct work_struct	update_tim_work;
 
 	int			beacon_int;
-- 
2.25.1

[PATCH 13/32] staging: wfx: drop argument tx_allowed_mask since it is constant now

[Jerome Pouiller]

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

Following the remove of asleep_mask, the tx_allowed_mask argument passed
to various functions is now always the same. Drop this argument and
simplify the code.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 48 +++++++++----------------------------
 drivers/staging/wfx/queue.h |  2 +-
 2 files changed, 12 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index e66debd60e3f..cecf9aa7b3ca 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -144,22 +144,15 @@ void wfx_tx_queues_deinit(struct wfx_dev *wdev)
 	wfx_tx_queues_clear(wdev);
 }
 
-int wfx_tx_queue_get_num_queued(struct wfx_queue *queue, u32 link_id_map)
+int wfx_tx_queue_get_num_queued(struct wfx_queue *queue)
 {
 	int ret, i;
 
-	if (!link_id_map)
-		return 0;
-
+	ret = 0;
 	spin_lock_bh(&queue->queue.lock);
-	if (link_id_map == (u32)-1) {
-		ret = skb_queue_len(&queue->queue);
-	} else {
-		ret = 0;
-		for (i = 0; i < ARRAY_SIZE(queue->link_map_cache); i++)
-			if (link_id_map & BIT(i))
-				ret += queue->link_map_cache[i];
-	}
+	for (i = 0; i < ARRAY_SIZE(queue->link_map_cache); i++)
+		if (i != WFX_LINK_ID_AFTER_DTIM)
+			ret += queue->link_map_cache[i];
 	spin_unlock_bh(&queue->queue.lock);
 	return ret;
 }
@@ -354,7 +347,7 @@ static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
 	}
 }
 
-static int wfx_get_prio_queue(struct wfx_vif *wvif, u32 tx_allowed_mask)
+static struct wfx_queue *wfx_tx_queue_mask_get(struct wfx_vif *wvif)
 {
 	const struct ieee80211_tx_queue_params *edca;
 	unsigned int score, best = -1;
@@ -366,8 +359,7 @@ static int wfx_get_prio_queue(struct wfx_vif *wvif, u32 tx_allowed_mask)
 		int queued;
 
 		edca = &wvif->edca_params[i];
-		queued = wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i],
-				tx_allowed_mask);
+		queued = wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i]);
 		if (!queued)
 			continue;
 		score = ((edca->aifs + edca->cw_min) << 16) +
@@ -379,23 +371,9 @@ static int wfx_get_prio_queue(struct wfx_vif *wvif, u32 tx_allowed_mask)
 		}
 	}
 
-	return winner;
-}
-
-static struct wfx_queue *wfx_tx_queue_mask_get(struct wfx_vif *wvif,
-					       u32 *tx_allowed_mask_p)
-{
-	int idx;
-	u32 tx_allowed_mask;
-
-	tx_allowed_mask = BIT(WFX_LINK_ID_MAX) - 1;
-	tx_allowed_mask &= ~BIT(WFX_LINK_ID_AFTER_DTIM);
-	idx = wfx_get_prio_queue(wvif, tx_allowed_mask);
-	if (idx < 0)
+	if (winner < 0)
 		return NULL;
-
-	*tx_allowed_mask_p = tx_allowed_mask;
-	return &wvif->wdev->tx_queue[idx];
+	return &wvif->wdev->tx_queue[winner];
 }
 
 struct hif_msg *wfx_tx_queues_get_after_dtim(struct wfx_vif *wvif)
@@ -424,8 +402,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 	struct hif_msg *hif = NULL;
 	struct wfx_queue *queue = NULL;
 	struct wfx_queue *vif_queue = NULL;
-	u32 tx_allowed_mask = 0;
-	u32 vif_tx_allowed_mask = 0;
 	struct wfx_vif *wvif;
 	int i;
 
@@ -459,12 +435,10 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 
 		wvif = NULL;
 		while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
-			vif_queue = wfx_tx_queue_mask_get(wvif,
-							  &vif_tx_allowed_mask);
+			vif_queue = wfx_tx_queue_mask_get(wvif);
 			if (vif_queue) {
 				if (queue && queue != vif_queue)
 					dev_info(wdev->dev, "vifs disagree about queue priority\n");
-				tx_allowed_mask |= vif_tx_allowed_mask;
 				queue = vif_queue;
 				ret = 0;
 			}
@@ -475,7 +449,7 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 
 		queue_num = queue - wdev->tx_queue;
 
-		skb = wfx_tx_queue_get(wdev, queue, tx_allowed_mask);
+		skb = wfx_tx_queue_get(wdev, queue, ~BIT(WFX_LINK_ID_AFTER_DTIM));
 		if (!skb)
 			continue;
 
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 5a5aa38dbb2f..58da216d47dd 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -47,7 +47,7 @@ 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);
-int wfx_tx_queue_get_num_queued(struct wfx_queue *queue, u32 link_id_map);
+int wfx_tx_queue_get_num_queued(struct wfx_queue *queue);
 
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
 int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb);
-- 
2.25.1

[PATCH 14/32] staging: wfx: do not use link_map_cache to track CAB

[Jerome Pouiller]

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

Since we do not track power save status of the stations anymore,
link_map_cache is now only used to track "Content After (DTIM) Beacon".
We prefer to rely on flags from tx_info. So we will be able to drop
link_map_cache.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index cecf9aa7b3ca..6fa8f4e083d3 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -146,13 +146,16 @@ void wfx_tx_queues_deinit(struct wfx_dev *wdev)
 
 int wfx_tx_queue_get_num_queued(struct wfx_queue *queue)
 {
-	int ret, i;
+	struct ieee80211_tx_info *tx_info;
+	struct sk_buff *skb;
+	int ret = 0;
 
-	ret = 0;
 	spin_lock_bh(&queue->queue.lock);
-	for (i = 0; i < ARRAY_SIZE(queue->link_map_cache); i++)
-		if (i != WFX_LINK_ID_AFTER_DTIM)
-			ret += queue->link_map_cache[i];
+	skb_queue_walk(&queue->queue, skb) {
+		tx_info = IEEE80211_SKB_CB(skb);
+		if (!(tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM))
+			ret++;
+	}
 	spin_unlock_bh(&queue->queue.lock);
 	return ret;
 }
-- 
2.25.1

[PATCH 15/32] staging: wfx: drop useless link_map_cache

[Jerome Pouiller]

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

Driver does not use link_map_cache anymore. So we can drop it. In add,
we do not have to keep this counter in sync with skb_queue item, so we
can drop explicit spin_locks.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 60 ++++++++-----------------------------
 drivers/staging/wfx/queue.h |  3 --
 2 files changed, 12 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 6fa8f4e083d3..82c2781b1f78 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -94,20 +94,10 @@ void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif)
 static void wfx_tx_queue_clear(struct wfx_dev *wdev, struct wfx_queue *queue,
 			       struct sk_buff_head *gc_list)
 {
-	int i;
 	struct sk_buff *item;
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 
-	spin_lock_bh(&queue->queue.lock);
-	while ((item = __skb_dequeue(&queue->queue)) != NULL)
+	while ((item = skb_dequeue(&queue->queue)) != NULL)
 		skb_queue_head(gc_list, item);
-	spin_lock_nested(&stats->pending.lock, 1);
-	for (i = 0; i < ARRAY_SIZE(stats->link_map_cache); ++i) {
-		stats->link_map_cache[i] -= queue->link_map_cache[i];
-		queue->link_map_cache[i] = 0;
-	}
-	spin_unlock(&stats->pending.lock);
-	spin_unlock_bh(&queue->queue.lock);
 }
 
 void wfx_tx_queues_clear(struct wfx_dev *wdev)
@@ -163,28 +153,15 @@ int wfx_tx_queue_get_num_queued(struct wfx_queue *queue)
 void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
 		      struct sk_buff *skb)
 {
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
-	struct wfx_tx_priv *tx_priv = wfx_skb_tx_priv(skb);
-
-	WARN(tx_priv->link_id >= ARRAY_SIZE(stats->link_map_cache), "invalid link-id value");
-	spin_lock_bh(&queue->queue.lock);
-	__skb_queue_tail(&queue->queue, skb);
-
-	++queue->link_map_cache[tx_priv->link_id];
-
-	spin_lock_nested(&stats->pending.lock, 1);
-	++stats->link_map_cache[tx_priv->link_id];
-	spin_unlock(&stats->pending.lock);
-	spin_unlock_bh(&queue->queue.lock);
+	skb_queue_tail(&queue->queue, skb);
 }
 
 static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 					struct wfx_queue *queue,
 					u32 link_id_map)
 {
-	struct sk_buff *skb = NULL;
-	struct sk_buff *item;
 	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
+	struct sk_buff *item, *skb = NULL;
 	struct wfx_tx_priv *tx_priv;
 
 	spin_lock_bh(&queue->queue.lock);
@@ -195,39 +172,28 @@ static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 			break;
 		}
 	}
+	spin_unlock_bh(&queue->queue.lock);
 	if (skb) {
+		skb_unlink(skb, &queue->queue);
 		tx_priv = wfx_skb_tx_priv(skb);
 		tx_priv->xmit_timestamp = ktime_get();
-		__skb_unlink(skb, &queue->queue);
-		--queue->link_map_cache[tx_priv->link_id];
-
-		spin_lock_nested(&stats->pending.lock, 1);
-		__skb_queue_tail(&stats->pending, skb);
-		--stats->link_map_cache[tx_priv->link_id];
-		spin_unlock(&stats->pending.lock);
+		skb_queue_tail(&stats->pending, skb);
+		if (skb_queue_empty(&queue->queue))
+			wake_up(&stats->wait_link_id_empty);
+		return skb;
 	}
-	spin_unlock_bh(&queue->queue.lock);
-	if (skb_queue_empty(&queue->queue))
-		wake_up(&stats->wait_link_id_empty);
 	return skb;
 }
 
 int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 {
 	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
-	struct wfx_tx_priv *tx_priv = wfx_skb_tx_priv(skb);
 	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
 
 	WARN_ON(skb_get_queue_mapping(skb) > 3);
-	spin_lock_bh(&queue->queue.lock);
-	++queue->link_map_cache[tx_priv->link_id];
 
-	spin_lock_nested(&stats->pending.lock, 1);
-	++stats->link_map_cache[tx_priv->link_id];
-	__skb_unlink(skb, &stats->pending);
-	spin_unlock(&stats->pending.lock);
-	__skb_queue_tail(&queue->queue, skb);
-	spin_unlock_bh(&queue->queue.lock);
+	skb_unlink(skb, &stats->pending);
+	skb_queue_tail(&queue->queue, skb);
 	return 0;
 }
 
@@ -235,9 +201,7 @@ int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb)
 {
 	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 
-	spin_lock_bh(&stats->pending.lock);
-	__skb_unlink(skb, &stats->pending);
-	spin_unlock_bh(&stats->pending.lock);
+	skb_unlink(skb, &stats->pending);
 	wfx_skb_dtor(wdev, skb);
 
 	return 0;
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 58da216d47dd..dd141cb4bf63 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -16,18 +16,15 @@
 #define WFX_LINK_ID_NO_ASSOC      15
 #define WFX_LINK_ID_AFTER_DTIM    (WFX_LINK_ID_NO_ASSOC + 1)
 #define WFX_LINK_ID_UAPSD         (WFX_LINK_ID_NO_ASSOC + 2)
-#define WFX_LINK_ID_MAX           (WFX_LINK_ID_NO_ASSOC + 3)
 
 struct wfx_dev;
 struct wfx_vif;
 
 struct wfx_queue {
 	struct sk_buff_head	queue;
-	int			link_map_cache[WFX_LINK_ID_MAX];
 };
 
 struct wfx_queue_stats {
-	int			link_map_cache[WFX_LINK_ID_MAX];
 	struct sk_buff_head	pending;
 	wait_queue_head_t	wait_link_id_empty;
 };
-- 
2.25.1

[PATCH 16/32] staging: wfx: do not rely anymore on link_id to choose packet in queue

[Jerome Pouiller]

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

link_id was expected to contain identifier of a station. It was also
used to mark frames that has to sent after dtim. We do not use the
further purpose. For the last purpose, we can directly check the flag
value in tx_info.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 82c2781b1f78..046aba77618a 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -158,16 +158,17 @@ void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
 
 static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 					struct wfx_queue *queue,
-					u32 link_id_map)
+					bool mcast)
 {
 	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
+	struct ieee80211_tx_info *tx_info;
 	struct sk_buff *item, *skb = NULL;
 	struct wfx_tx_priv *tx_priv;
 
 	spin_lock_bh(&queue->queue.lock);
 	skb_queue_walk(&queue->queue, item) {
-		tx_priv = wfx_skb_tx_priv(item);
-		if (link_id_map & BIT(tx_priv->link_id)) {
+		tx_info = IEEE80211_SKB_CB(item);
+		if (mcast == !!(tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)) {
 			skb = item;
 			break;
 		}
@@ -381,7 +382,7 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 			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));
+						       true);
 				if (skb) {
 					hif = (struct hif_msg *)skb->data;
 					// Cannot happen since only one vif can
@@ -416,7 +417,7 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 
 		queue_num = queue - wdev->tx_queue;
 
-		skb = wfx_tx_queue_get(wdev, queue, ~BIT(WFX_LINK_ID_AFTER_DTIM));
+		skb = wfx_tx_queue_get(wdev, queue, false);
 		if (!skb)
 			continue;
 
-- 
2.25.1

[PATCH 17/32] staging: wfx: drop unused link_id field

[Jerome Pouiller]

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

It is not used anymore.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 5 -----
 drivers/staging/wfx/data_tx.h | 1 -
 drivers/staging/wfx/queue.h   | 2 --
 3 files changed, 8 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index a53e6d15031b..f794212f42e2 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -424,13 +424,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	// Fill tx_priv
 	tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
 	tx_priv->raw_link_id = wfx_tx_get_raw_link_id(wvif, sta, hdr);
-	tx_priv->link_id = tx_priv->raw_link_id;
 	if (ieee80211_has_protected(hdr->frame_control))
 		tx_priv->hw_key = hw_key;
-	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
-		tx_priv->link_id = WFX_LINK_ID_AFTER_DTIM;
-	if (sta && (sta->uapsd_queues & BIT(queue_id)))
-		tx_priv->link_id = WFX_LINK_ID_UAPSD;
 
 	// Fill hif_msg
 	WARN(skb_headroom(skb) < wmsg_len, "not enough space in skb");
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index c545dd75449b..b561bbf9f16f 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -36,7 +36,6 @@ struct tx_policy_cache {
 struct wfx_tx_priv {
 	ktime_t xmit_timestamp;
 	struct ieee80211_key_conf *hw_key;
-	u8 link_id;
 	u8 raw_link_id;
 } __packed;
 
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index dd141cb4bf63..39c265e4b86e 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -14,8 +14,6 @@
 
 #define WFX_MAX_STA_IN_AP_MODE    14
 #define WFX_LINK_ID_NO_ASSOC      15
-#define WFX_LINK_ID_AFTER_DTIM    (WFX_LINK_ID_NO_ASSOC + 1)
-#define WFX_LINK_ID_UAPSD         (WFX_LINK_ID_NO_ASSOC + 2)
 
 struct wfx_dev;
 struct wfx_vif;
-- 
2.25.1

[PATCH 18/32] staging: wfx: drop unused raw_link_id field

[Jerome Pouiller]

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

raw_link_id can be retrieved by wfx_tx_get_raw_link_id(). So, it is not
necessary to keep it in struct wfx_tx_priv.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 3 +--
 drivers/staging/wfx/data_tx.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index f794212f42e2..57afabc102a7 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -423,7 +423,6 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	memset(tx_info->rate_driver_data, 0, sizeof(struct wfx_tx_priv));
 	// Fill tx_priv
 	tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
-	tx_priv->raw_link_id = wfx_tx_get_raw_link_id(wvif, sta, hdr);
 	if (ieee80211_has_protected(hdr->frame_control))
 		tx_priv->hw_key = hw_key;
 
@@ -455,7 +454,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	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;
+	req->queue_id.peer_sta_id = wfx_tx_get_raw_link_id(wvif, sta, hdr);
 	// Queue index are inverted between firmware and Linux
 	req->queue_id.queue_id = 3 - queue_id;
 	req->ht_tx_parameters = wfx_tx_get_tx_parms(wvif->wdev, tx_info);
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index b561bbf9f16f..03fe3e319ba1 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -36,7 +36,6 @@ struct tx_policy_cache {
 struct wfx_tx_priv {
 	ktime_t xmit_timestamp;
 	struct ieee80211_key_conf *hw_key;
-	u8 raw_link_id;
 } __packed;
 
 void wfx_tx_policy_init(struct wfx_vif *wvif);
-- 
2.25.1

[PATCH 19/32] staging: wfx: rename wfx_tx_get_raw_link_id()

[Jerome Pouiller]

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

Since concept of "raw_link_id" does not exist anymore, rename
wfx_tx_get_raw_link_id() in wfx_tx_get_link_id().

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 57afabc102a7..2533d4f53f83 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -287,9 +287,8 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 	}
 }
 
-static u8 wfx_tx_get_raw_link_id(struct wfx_vif *wvif,
-				 struct ieee80211_sta *sta,
-				 struct ieee80211_hdr *hdr)
+static u8 wfx_tx_get_link_id(struct wfx_vif *wvif, struct ieee80211_sta *sta,
+			     struct ieee80211_hdr *hdr)
 {
 	struct wfx_sta_priv *sta_priv =
 		sta ? (struct wfx_sta_priv *)&sta->drv_priv : NULL;
@@ -454,7 +453,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	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 = wfx_tx_get_raw_link_id(wvif, sta, hdr);
+	req->queue_id.peer_sta_id = wfx_tx_get_link_id(wvif, sta, hdr);
 	// Queue index are inverted between firmware and Linux
 	req->queue_id.queue_id = 3 - queue_id;
 	req->ht_tx_parameters = wfx_tx_get_tx_parms(wvif->wdev, tx_info);
-- 
2.25.1

[PATCH 20/32] staging: wfx: replace wfx_tx_queues_get_after_dtim() by wfx_tx_queues_has_cab()

[Jerome Pouiller]

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

It is not necessary to return a skb. Just getting the information if
there is traffic to be sent after DTIM is sufficient.

In add, the acronym "cab" (Content After (DTIM) Beacon) is used in
mac80211 to designate this kind of traffic.

So, make wfx_tx_queues_get_after_dtim() return a boolean and rename
accordingly.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 40 ++++++++++++++++++-------------------
 drivers/staging/wfx/queue.h |  2 +-
 drivers/staging/wfx/sta.c   |  4 ++--
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 046aba77618a..4ddb2c7370cd 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -264,6 +264,26 @@ unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 	return ktime_us_delta(now, tx_priv->xmit_timestamp);
 }
 
+bool wfx_tx_queues_has_cab(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 true;
+		}
+	}
+	return false;
+}
+
 bool wfx_tx_queues_empty(struct wfx_dev *wdev)
 {
 	int i;
@@ -344,26 +364,6 @@ static struct wfx_queue *wfx_tx_queue_mask_get(struct wfx_vif *wvif)
 	return &wvif->wdev->tx_queue[winner];
 }
 
-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;
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 39c265e4b86e..2c4724699ed0 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -36,9 +36,9 @@ void wfx_tx_queues_init(struct wfx_dev *wdev);
 void wfx_tx_queues_deinit(struct wfx_dev *wdev);
 void wfx_tx_queues_clear(struct wfx_dev *wdev);
 bool wfx_tx_queues_empty(struct wfx_dev *wdev);
+bool wfx_tx_queues_has_cab(struct wfx_vif *wvif);
 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 5c5b52dc7bdd..e1d7a0670c9d 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -857,7 +857,7 @@ static int wfx_update_tim(struct wfx_vif *wvif)
 		tim_ptr[2] = 0;
 
 		/* Set/reset aid0 bit */
-		if (wfx_tx_queues_get_after_dtim(wvif))
+		if (wfx_tx_queues_has_cab(wvif))
 			tim_ptr[4] |= 1;
 		else
 			tim_ptr[4] &= ~1;
@@ -888,7 +888,7 @@ int wfx_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta, bool set)
 
 void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd)
 {
-	WARN(!wfx_tx_queues_get_after_dtim(wvif), "incorrect sequence");
+	WARN(!wfx_tx_queues_has_cab(wvif), "incorrect sequence");
 	WARN(wvif->after_dtim_tx_allowed, "incorrect sequence");
 	wvif->after_dtim_tx_allowed = true;
 	wfx_bh_request_tx(wvif->wdev);
-- 
2.25.1

[PATCH 21/32] staging: wfx: introduce a counter of pending frames

[Jerome Pouiller]

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

This counter will be useful to know which queue is least full in a
further patch.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 15 +++++++++++++++
 drivers/staging/wfx/queue.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 4ddb2c7370cd..21a2c8aabbb9 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -30,6 +30,7 @@ void wfx_tx_unlock(struct wfx_dev *wdev)
 void wfx_tx_flush(struct wfx_dev *wdev)
 {
 	int ret;
+	int i;
 
 	// Do not wait for any reply if chip is frozen
 	if (wdev->chip_frozen)
@@ -39,6 +40,12 @@ void wfx_tx_flush(struct wfx_dev *wdev)
 	ret = wait_event_timeout(wdev->hif.tx_buffers_empty,
 				 !wdev->hif.tx_buffers_used,
 				 msecs_to_jiffies(3000));
+	if (ret) {
+		for (i = 0; i < IEEE80211_NUM_ACS; i++)
+			WARN(atomic_read(&wdev->tx_queue[i].pending_frames),
+			     "there are still %d pending frames on queue %d",
+			     atomic_read(&wdev->tx_queue[i].pending_frames), i);
+	}
 	if (!ret) {
 		dev_warn(wdev->dev, "cannot flush tx buffers (%d still busy)\n",
 			 wdev->hif.tx_buffers_used);
@@ -176,6 +183,7 @@ static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
 	spin_unlock_bh(&queue->queue.lock);
 	if (skb) {
 		skb_unlink(skb, &queue->queue);
+		atomic_inc(&queue->pending_frames);
 		tx_priv = wfx_skb_tx_priv(skb);
 		tx_priv->xmit_timestamp = ktime_get();
 		skb_queue_tail(&stats->pending, skb);
@@ -192,7 +200,9 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
 
 	WARN_ON(skb_get_queue_mapping(skb) > 3);
+	WARN_ON(!atomic_read(&queue->pending_frames));
 
+	atomic_dec(&queue->pending_frames);
 	skb_unlink(skb, &stats->pending);
 	skb_queue_tail(&queue->queue, skb);
 	return 0;
@@ -201,7 +211,12 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb)
 {
 	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
+	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
 
+	WARN_ON(skb_get_queue_mapping(skb) > 3);
+	WARN_ON(!atomic_read(&queue->pending_frames));
+
+	atomic_dec(&queue->pending_frames);
 	skb_unlink(skb, &stats->pending);
 	wfx_skb_dtor(wdev, skb);
 
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 2c4724699ed0..c24b8cd41a78 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -9,6 +9,7 @@
 #define WFX_QUEUE_H
 
 #include <linux/skbuff.h>
+#include <linux/atomic.h>
 
 #include "hif_api_cmd.h"
 
@@ -20,6 +21,7 @@ struct wfx_vif;
 
 struct wfx_queue {
 	struct sk_buff_head	queue;
+	atomic_t		pending_frames;
 };
 
 struct wfx_queue_stats {
-- 
2.25.1

[PATCH 22/32] staging: wfx: change the way to choose frame to send

[Jerome Pouiller]

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

The current code computes itself the QoS policy to choose which frame
should be sent. However, firmware already do that job. Firmware would
prefer to have packets in every queues and be able to choose itself
which queue to use.

So, this patch sort the queues from the emptiest to the fulliest (thanks
to the pending frames counter introduced a few commits earlier). It send
frame to the least full queue.

However, we continue to be careful with frames that have to be sent
after a dtim ("cab": Content After (DTIM) Beacon).

So, this patch splits AC queues in two skb_queues: one for normal frames
and another for cab frames. It cares to send frames from CAB skb_queue
if appropriate.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 238 ++++++++++++++----------------------
 drivers/staging/wfx/queue.h |   4 +-
 2 files changed, 94 insertions(+), 148 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 21a2c8aabbb9..b45fb837f1cd 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -83,13 +83,20 @@ void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif)
 		wfx_tx_lock_flush(wdev);
 		for (i = 0; i < IEEE80211_NUM_ACS && done; ++i) {
 			queue = &wdev->tx_queue[i];
-			spin_lock_bh(&queue->queue.lock);
-			skb_queue_walk(&queue->queue, item) {
-				hif = (struct hif_msg *) item->data;
+			spin_lock_bh(&queue->normal.lock);
+			skb_queue_walk(&queue->normal, item) {
+				hif = (struct hif_msg *)item->data;
 				if (hif->interface == wvif->id)
 					done = false;
 			}
-			spin_unlock_bh(&queue->queue.lock);
+			spin_unlock_bh(&queue->normal.lock);
+			spin_lock_bh(&queue->cab.lock);
+			skb_queue_walk(&queue->cab, item) {
+				hif = (struct hif_msg *)item->data;
+				if (hif->interface == wvif->id)
+					done = false;
+			}
+			spin_unlock_bh(&queue->cab.lock);
 		}
 		if (!done) {
 			wfx_tx_unlock(wdev);
@@ -103,7 +110,9 @@ static void wfx_tx_queue_clear(struct wfx_dev *wdev, struct wfx_queue *queue,
 {
 	struct sk_buff *item;
 
-	while ((item = skb_dequeue(&queue->queue)) != NULL)
+	while ((item = skb_dequeue(&queue->normal)) != NULL)
+		skb_queue_head(gc_list, item);
+	while ((item = skb_dequeue(&queue->cab)) != NULL)
 		skb_queue_head(gc_list, item);
 }
 
@@ -131,8 +140,10 @@ void wfx_tx_queues_init(struct wfx_dev *wdev)
 	skb_queue_head_init(&wdev->tx_queue_stats.pending);
 	init_waitqueue_head(&wdev->tx_queue_stats.wait_link_id_empty);
 
-	for (i = 0; i < IEEE80211_NUM_ACS; ++i)
-		skb_queue_head_init(&wdev->tx_queue[i].queue);
+	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
+		skb_queue_head_init(&wdev->tx_queue[i].normal);
+		skb_queue_head_init(&wdev->tx_queue[i].cab);
+	}
 }
 
 void wfx_tx_queues_deinit(struct wfx_dev *wdev)
@@ -141,57 +152,15 @@ void wfx_tx_queues_deinit(struct wfx_dev *wdev)
 	wfx_tx_queues_clear(wdev);
 }
 
-int wfx_tx_queue_get_num_queued(struct wfx_queue *queue)
-{
-	struct ieee80211_tx_info *tx_info;
-	struct sk_buff *skb;
-	int ret = 0;
-
-	spin_lock_bh(&queue->queue.lock);
-	skb_queue_walk(&queue->queue, skb) {
-		tx_info = IEEE80211_SKB_CB(skb);
-		if (!(tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM))
-			ret++;
-	}
-	spin_unlock_bh(&queue->queue.lock);
-	return ret;
-}
-
 void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
 		      struct sk_buff *skb)
 {
-	skb_queue_tail(&queue->queue, skb);
-}
+	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 
-static struct sk_buff *wfx_tx_queue_get(struct wfx_dev *wdev,
-					struct wfx_queue *queue,
-					bool mcast)
-{
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
-	struct ieee80211_tx_info *tx_info;
-	struct sk_buff *item, *skb = NULL;
-	struct wfx_tx_priv *tx_priv;
-
-	spin_lock_bh(&queue->queue.lock);
-	skb_queue_walk(&queue->queue, item) {
-		tx_info = IEEE80211_SKB_CB(item);
-		if (mcast == !!(tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)) {
-			skb = item;
-			break;
-		}
-	}
-	spin_unlock_bh(&queue->queue.lock);
-	if (skb) {
-		skb_unlink(skb, &queue->queue);
-		atomic_inc(&queue->pending_frames);
-		tx_priv = wfx_skb_tx_priv(skb);
-		tx_priv->xmit_timestamp = ktime_get();
-		skb_queue_tail(&stats->pending, skb);
-		if (skb_queue_empty(&queue->queue))
-			wake_up(&stats->wait_link_id_empty);
-		return skb;
-	}
-	return skb;
+	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
+		skb_queue_tail(&queue->cab, skb);
+	else
+		skb_queue_tail(&queue->normal, skb);
 }
 
 int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
@@ -204,7 +173,7 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 
 	atomic_dec(&queue->pending_frames);
 	skb_unlink(skb, &stats->pending);
-	skb_queue_tail(&queue->queue, skb);
+	wfx_tx_queue_put(wdev, queue, skb);
 	return 0;
 }
 
@@ -282,20 +251,15 @@ unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 bool wfx_tx_queues_has_cab(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 true;
-		}
-	}
+	if (wvif->vif->type != NL80211_IFTYPE_AP)
+		return false;
+	for (i = 0; i < IEEE80211_NUM_ACS; ++i)
+		// Note: since only AP can have mcast frames in queue and only
+		// one vif can be AP, all queued frames has same interface id
+		if (!skb_queue_empty_lockless(&wdev->tx_queue[i].cab))
+			return true;
 	return false;
 }
 
@@ -304,7 +268,8 @@ bool wfx_tx_queues_empty(struct wfx_dev *wdev)
 	int i;
 
 	for (i = 0; i < IEEE80211_NUM_ACS; i++)
-		if (!skb_queue_empty_lockless(&wdev->tx_queue[i].queue))
+		if (!skb_queue_empty_lockless(&wdev->tx_queue[i].normal) ||
+		    !skb_queue_empty_lockless(&wdev->tx_queue[i].cab))
 			return false;
 	return true;
 }
@@ -350,95 +315,76 @@ static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
 	}
 }
 
-static struct wfx_queue *wfx_tx_queue_mask_get(struct wfx_vif *wvif)
+static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 {
-	const struct ieee80211_tx_queue_params *edca;
-	unsigned int score, best = -1;
-	int winner = -1;
-	int i;
-
-	/* search for a winner using edca params */
-	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
-		int queued;
-
-		edca = &wvif->edca_params[i];
-		queued = wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i]);
-		if (!queued)
-			continue;
-		score = ((edca->aifs + edca->cw_min) << 16) +
-			((edca->cw_max - edca->cw_min) *
-			 (get_random_int() & 0xFFFF));
-		if (score < best && (winner < 0 || i != 3)) {
-			best = score;
-			winner = i;
-		}
-	}
-
-	if (winner < 0)
-		return NULL;
-	return &wvif->wdev->tx_queue[winner];
-}
-
-struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
-{
-	struct sk_buff *skb;
-	struct hif_msg *hif = NULL;
-	struct wfx_queue *queue = NULL;
-	struct wfx_queue *vif_queue = NULL;
+	struct wfx_queue *sorted_queues[IEEE80211_NUM_ACS];
 	struct wfx_vif *wvif;
-	int i;
-
-	if (atomic_read(&wdev->tx_lock))
-		return NULL;
+	struct hif_msg *hif;
+	struct sk_buff *skb;
+	int i, j;
 
+	// bubble sort
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		sorted_queues[i] = &wdev->tx_queue[i];
+		for (j = i; j > 0; j--)
+			if (atomic_read(&sorted_queues[j]->pending_frames) >
+			    atomic_read(&sorted_queues[j - 1]->pending_frames))
+				swap(sorted_queues[j - 1], sorted_queues[j]);
+	}
 	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],
-						       true);
-				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);
+		if (!wvif->after_dtim_tx_allowed)
+			continue;
+		for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+			skb = skb_dequeue(&sorted_queues[i]->cab);
+			if (!skb)
+				continue;
+			// Note: since only AP can have mcast frames in queue
+			// and only one vif can be AP, all queued frames has
+			// same interface id
+			hif = (struct hif_msg *)skb->data;
+			WARN_ON(hif->interface != wvif->id);
+			WARN_ON(sorted_queues[i] !=
+				&wdev->tx_queue[skb_get_queue_mapping(skb)]);
+			atomic_inc(&sorted_queues[i]->pending_frames);
+			return skb;
 		}
+		// No more multicast to sent
+		wvif->after_dtim_tx_allowed = false;
+		schedule_work(&wvif->update_tim_work);
 	}
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		skb = skb_dequeue(&sorted_queues[i]->normal);
+		if (skb) {
+			WARN_ON(sorted_queues[i] !=
+				&wdev->tx_queue[skb_get_queue_mapping(skb)]);
+			atomic_inc(&sorted_queues[i]->pending_frames);
+			return skb;
+		}
+	}
+	return NULL;
+}
+
+struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
+{
+	struct wfx_tx_priv *tx_priv;
+	struct sk_buff *skb;
+
+	if (atomic_read(&wdev->tx_lock))
+		return NULL;
 
 	for (;;) {
-		int ret = -ENOENT;
-		int queue_num;
-
-		wvif = NULL;
-		while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
-			vif_queue = wfx_tx_queue_mask_get(wvif);
-			if (vif_queue) {
-				if (queue && queue != vif_queue)
-					dev_info(wdev->dev, "vifs disagree about queue priority\n");
-				queue = vif_queue;
-				ret = 0;
-			}
-		}
-
-		if (ret)
-			return NULL;
-
-		queue_num = queue - wdev->tx_queue;
-
-		skb = wfx_tx_queue_get(wdev, queue, false);
+		skb = wfx_tx_queues_get_skb(wdev);
 		if (!skb)
-			continue;
-
+			return NULL;
+		skb_queue_tail(&wdev->tx_queue_stats.pending, skb);
+		if (wfx_tx_queues_empty(wdev))
+			wake_up(&wdev->tx_queue_stats.wait_link_id_empty);
+		// FIXME: is it useful?
 		if (wfx_handle_tx_data(wdev, skb))
-			continue;  /* Handled by WSM */
-
+			continue;
+		tx_priv = wfx_skb_tx_priv(skb);
+		tx_priv->xmit_timestamp = ktime_get();
 		return (struct hif_msg *)skb->data;
 	}
 }
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index c24b8cd41a78..8e99bb2792ed 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -20,7 +20,8 @@ struct wfx_dev;
 struct wfx_vif;
 
 struct wfx_queue {
-	struct sk_buff_head	queue;
+	struct sk_buff_head	normal;
+	struct sk_buff_head	cab; // Content After (DTIM) Beacon
 	atomic_t		pending_frames;
 };
 
@@ -44,7 +45,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev);
 
 void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
 		      struct sk_buff *skb);
-int wfx_tx_queue_get_num_queued(struct wfx_queue *queue);
 
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
 int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb);
-- 
2.25.1

[PATCH 23/32] staging: wfx: drop now useless field edca_params

[Jerome Pouiller]

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

Since we do not rely in QoS parameters to choose which frame to send, it
is no more necessary to keep a copy of EDCA parameters.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 1 -
 drivers/staging/wfx/wfx.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index e1d7a0670c9d..15f00ea84068 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -298,7 +298,6 @@ int wfx_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	mutex_lock(&wdev->conf_mutex);
 	assign_bit(queue, &wvif->uapsd_mask, params->uapsd);
-	memcpy(&wvif->edca_params[queue], params, sizeof(*params));
 	hif_set_edca_queue_params(wvif, queue, params);
 	if (wvif->vif->type == NL80211_IFTYPE_STATION &&
 	    old_uapsd != wvif->uapsd_mask) {
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 61899cd7942b..6b5b95a45e60 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -94,7 +94,6 @@ struct wfx_vif {
 	struct work_struct	update_filtering_work;
 
 	unsigned long		uapsd_mask;
-	struct ieee80211_tx_queue_params edca_params[IEEE80211_NUM_ACS];
 	struct hif_req_set_bss_params bss_params;
 	struct work_struct	bss_params_work;
 
-- 
2.25.1

[PATCH 24/32] staging: wfx: drop struct wfx_queue_stats

[Jerome Pouiller]

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

There is no reason to keep the intermediate struct wfx_queue_stats.
Relocate its members to struct wfx_dev.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 36 +++++++++++++++---------------------
 drivers/staging/wfx/queue.h |  5 -----
 drivers/staging/wfx/sta.c   |  2 +-
 drivers/staging/wfx/wfx.h   |  3 ++-
 4 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index b45fb837f1cd..a03860db2f54 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -121,12 +121,11 @@ void wfx_tx_queues_clear(struct wfx_dev *wdev)
 	int i;
 	struct sk_buff *item;
 	struct sk_buff_head gc_list;
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 
 	skb_queue_head_init(&gc_list);
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i)
 		wfx_tx_queue_clear(wdev, &wdev->tx_queue[i], &gc_list);
-	wake_up(&stats->wait_link_id_empty);
+	wake_up(&wdev->tx_dequeue);
 	while ((item = skb_dequeue(&gc_list)) != NULL)
 		wfx_skb_dtor(wdev, item);
 }
@@ -135,10 +134,9 @@ void wfx_tx_queues_init(struct wfx_dev *wdev)
 {
 	int i;
 
-	memset(&wdev->tx_queue_stats, 0, sizeof(wdev->tx_queue_stats));
 	memset(wdev->tx_queue, 0, sizeof(wdev->tx_queue));
-	skb_queue_head_init(&wdev->tx_queue_stats.pending);
-	init_waitqueue_head(&wdev->tx_queue_stats.wait_link_id_empty);
+	skb_queue_head_init(&wdev->tx_pending);
+	init_waitqueue_head(&wdev->tx_dequeue);
 
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
 		skb_queue_head_init(&wdev->tx_queue[i].normal);
@@ -148,7 +146,7 @@ void wfx_tx_queues_init(struct wfx_dev *wdev)
 
 void wfx_tx_queues_deinit(struct wfx_dev *wdev)
 {
-	WARN_ON(!skb_queue_empty(&wdev->tx_queue_stats.pending));
+	WARN_ON(!skb_queue_empty(&wdev->tx_pending));
 	wfx_tx_queues_clear(wdev);
 }
 
@@ -165,28 +163,26 @@ void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
 
 int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 {
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
 
 	WARN_ON(skb_get_queue_mapping(skb) > 3);
 	WARN_ON(!atomic_read(&queue->pending_frames));
 
 	atomic_dec(&queue->pending_frames);
-	skb_unlink(skb, &stats->pending);
+	skb_unlink(skb, &wdev->tx_pending);
 	wfx_tx_queue_put(wdev, queue, skb);
 	return 0;
 }
 
 int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb)
 {
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
 
 	WARN_ON(skb_get_queue_mapping(skb) > 3);
 	WARN_ON(!atomic_read(&queue->pending_frames));
 
 	atomic_dec(&queue->pending_frames);
-	skb_unlink(skb, &stats->pending);
+	skb_unlink(skb, &wdev->tx_pending);
 	wfx_skb_dtor(wdev, skb);
 
 	return 0;
@@ -196,32 +192,30 @@ struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
 {
 	struct sk_buff *skb;
 	struct hif_req_tx *req;
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 
-	spin_lock_bh(&stats->pending.lock);
-	skb_queue_walk(&stats->pending, skb) {
+	spin_lock_bh(&wdev->tx_pending.lock);
+	skb_queue_walk(&wdev->tx_pending, skb) {
 		req = wfx_skb_txreq(skb);
 		if (req->packet_id == packet_id) {
-			spin_unlock_bh(&stats->pending.lock);
+			spin_unlock_bh(&wdev->tx_pending.lock);
 			return skb;
 		}
 	}
-	spin_unlock_bh(&stats->pending.lock);
+	spin_unlock_bh(&wdev->tx_pending.lock);
 	WARN(1, "cannot find packet in pending queue");
 	return NULL;
 }
 
 void wfx_pending_dump_old_frames(struct wfx_dev *wdev, unsigned int limit_ms)
 {
-	struct wfx_queue_stats *stats = &wdev->tx_queue_stats;
 	ktime_t now = ktime_get();
 	struct wfx_tx_priv *tx_priv;
 	struct hif_req_tx *req;
 	struct sk_buff *skb;
 	bool first = true;
 
-	spin_lock_bh(&stats->pending.lock);
-	skb_queue_walk(&stats->pending, skb) {
+	spin_lock_bh(&wdev->tx_pending.lock);
+	skb_queue_walk(&wdev->tx_pending, skb) {
 		tx_priv = wfx_skb_tx_priv(skb);
 		req = wfx_skb_txreq(skb);
 		if (ktime_after(now, ktime_add_ms(tx_priv->xmit_timestamp,
@@ -236,7 +230,7 @@ void wfx_pending_dump_old_frames(struct wfx_dev *wdev, unsigned int limit_ms)
 				 ktime_ms_delta(now, tx_priv->xmit_timestamp));
 		}
 	}
-	spin_unlock_bh(&stats->pending.lock);
+	spin_unlock_bh(&wdev->tx_pending.lock);
 }
 
 unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
@@ -377,9 +371,9 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 		skb = wfx_tx_queues_get_skb(wdev);
 		if (!skb)
 			return NULL;
-		skb_queue_tail(&wdev->tx_queue_stats.pending, skb);
+		skb_queue_tail(&wdev->tx_pending, skb);
 		if (wfx_tx_queues_empty(wdev))
-			wake_up(&wdev->tx_queue_stats.wait_link_id_empty);
+			wake_up(&wdev->tx_dequeue);
 		// FIXME: is it useful?
 		if (wfx_handle_tx_data(wdev, skb))
 			continue;
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 8e99bb2792ed..241ca3039b54 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -25,11 +25,6 @@ struct wfx_queue {
 	atomic_t		pending_frames;
 };
 
-struct wfx_queue_stats {
-	struct sk_buff_head	pending;
-	wait_queue_head_t	wait_link_id_empty;
-};
-
 void wfx_tx_lock(struct wfx_dev *wdev);
 void wfx_tx_unlock(struct wfx_dev *wdev);
 void wfx_tx_flush(struct wfx_dev *wdev);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 15f00ea84068..340e09bb639d 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -323,7 +323,7 @@ static int __wfx_flush(struct wfx_dev *wdev, bool drop)
 	for (;;) {
 		if (drop)
 			wfx_tx_queues_clear(wdev);
-		if (wait_event_timeout(wdev->tx_queue_stats.wait_link_id_empty,
+		if (wait_event_timeout(wdev->tx_dequeue,
 				       wfx_tx_queues_empty(wdev),
 				       2 * HZ) <= 0)
 			return -ETIMEDOUT;
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 6b5b95a45e60..af4c93af81be 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -50,7 +50,8 @@ struct wfx_dev {
 
 	struct wfx_hif_cmd	hif_cmd;
 	struct wfx_queue	tx_queue[4];
-	struct wfx_queue_stats	tx_queue_stats;
+	struct sk_buff_head	tx_pending;
+	wait_queue_head_t	tx_dequeue;
 	atomic_t		tx_lock;
 
 	atomic_t		packet_id;
-- 
2.25.1

[PATCH 25/32] staging: wfx: simplify usage of wfx_tx_queues_put()

[Jerome Pouiller]

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

The queue used for wfx_tx_queue_put() can be deducted from the content
of the skb. So drop this parameter from call to wfx_tx_queues_put().

In add, this change uniformizes usage of functions wfx_tx_queues_*.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 2 +-
 drivers/staging/wfx/queue.c   | 6 +++---
 drivers/staging/wfx/queue.h   | 3 +--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 2533d4f53f83..d2e925218eda 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -461,7 +461,7 @@ 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);
+	wfx_tx_queues_put(wvif->wdev, skb);
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
 		schedule_work(&wvif->update_tim_work);
 	wfx_bh_request_tx(wvif->wdev);
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index a03860db2f54..cc89bfe1dbb4 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -150,9 +150,9 @@ void wfx_tx_queues_deinit(struct wfx_dev *wdev)
 	wfx_tx_queues_clear(wdev);
 }
 
-void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
-		      struct sk_buff *skb)
+void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb)
 {
+	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
@@ -170,7 +170,7 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 
 	atomic_dec(&queue->pending_frames);
 	skb_unlink(skb, &wdev->tx_pending);
-	wfx_tx_queue_put(wdev, queue, skb);
+	wfx_tx_queues_put(wdev, skb);
 	return 0;
 }
 
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 241ca3039b54..4851635d159b 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -36,10 +36,9 @@ void wfx_tx_queues_clear(struct wfx_dev *wdev);
 bool wfx_tx_queues_empty(struct wfx_dev *wdev);
 bool wfx_tx_queues_has_cab(struct wfx_vif *wvif);
 void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif);
+void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb);
 struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev);
 
-void wfx_tx_queue_put(struct wfx_dev *wdev, struct wfx_queue *queue,
-		      struct sk_buff *skb);
 
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
 int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb);
-- 
2.25.1

[PATCH 26/32] staging: wfx: improve interface between data_tx.c and queue.c

[Jerome Pouiller]

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

Currently, wfx_pending_remove() (from queue.c) call wfx_skb_dtor()
(from data_tx.c) that forward the tx status to mac80211.

Moreover, there no purpose to retrieve a frame from the pending queue
without dequeuing it. So, the main purpose of wfx_pending_remove() is to
forward the tx status to mac80211.

Let's make the architecture cleaner:
  - merge wfx_pending_remove() into wfx_pending_get()
  - call wfx_skb_dtor() from data_tx.c

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c |  2 +-
 drivers/staging/wfx/queue.c   | 22 +++++++---------------
 drivers/staging/wfx/queue.h   |  1 -
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index d2e925218eda..17209f645e4b 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -586,7 +586,7 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		    arg->packet_id == wvif->bss_loss_confirm_id)
 			wfx_cqm_bssloss_sm(wvif, 0, 0, 1);
 	}
-	wfx_pending_remove(wvif->wdev, skb);
+	wfx_skb_dtor(wvif->wdev, skb);
 }
 
 static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb)
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index cc89bfe1dbb4..a1a2f7756a27 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -174,30 +174,22 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 	return 0;
 }
 
-int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb)
-{
-	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
-
-	WARN_ON(skb_get_queue_mapping(skb) > 3);
-	WARN_ON(!atomic_read(&queue->pending_frames));
-
-	atomic_dec(&queue->pending_frames);
-	skb_unlink(skb, &wdev->tx_pending);
-	wfx_skb_dtor(wdev, skb);
-
-	return 0;
-}
-
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
 {
-	struct sk_buff *skb;
+	struct wfx_queue *queue;
 	struct hif_req_tx *req;
+	struct sk_buff *skb;
 
 	spin_lock_bh(&wdev->tx_pending.lock);
 	skb_queue_walk(&wdev->tx_pending, skb) {
 		req = wfx_skb_txreq(skb);
 		if (req->packet_id == packet_id) {
 			spin_unlock_bh(&wdev->tx_pending.lock);
+			queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
+			WARN_ON(skb_get_queue_mapping(skb) > 3);
+			WARN_ON(!atomic_read(&queue->pending_frames));
+			atomic_dec(&queue->pending_frames);
+			skb_unlink(skb, &wdev->tx_pending);
 			return skb;
 		}
 	}
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 4851635d159b..9bc1a5200e64 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -41,7 +41,6 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev);
 
 
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
-int wfx_pending_remove(struct wfx_dev *wdev, struct sk_buff *skb);
 int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb);
 unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 					  struct sk_buff *skb);
-- 
2.25.1

[PATCH 27/32] staging: wfx: relocate wfx_skb_dtor() prior its callers

[Jerome Pouiller]

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

In a next commit, we would like to mark wfx_skb_dtor as static and stop
to declare it in data_tx.h.

Relocate wfx_skb_dtor() prior its callers to avoid compile error.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 72 ++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 17209f645e4b..ec95518c9167 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -503,6 +503,43 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	ieee80211_tx_status_irqsafe(wdev->hw, skb);
 }
 
+static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb)
+{
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct ieee80211_sta *sta;
+	struct wfx_sta_priv *sta_priv;
+	int tid = ieee80211_get_tid(hdr);
+
+	rcu_read_lock(); // protect sta
+	sta = ieee80211_find_sta(wvif->vif, hdr->addr1);
+	if (sta) {
+		sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
+		spin_lock_bh(&sta_priv->lock);
+		WARN(!sta_priv->buffered[tid], "inconsistent notification");
+		sta_priv->buffered[tid]--;
+		if (!sta_priv->buffered[tid])
+			ieee80211_sta_set_buffered(sta, tid, false);
+		spin_unlock_bh(&sta_priv->lock);
+	}
+	rcu_read_unlock();
+}
+
+void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
+{
+	struct hif_msg *hif = (struct hif_msg *)skb->data;
+	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
+	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
+	unsigned int offset = sizeof(struct hif_req_tx) +
+				sizeof(struct hif_msg) +
+				req->data_flags.fc_offset;
+
+	WARN_ON(!wvif);
+	skb_pull(skb, offset);
+	wfx_notify_buffered_tx(wvif, skb);
+	wfx_tx_policy_put(wvif, req->tx_flags.retry_policy_index);
+	ieee80211_tx_status_irqsafe(wdev->hw, skb);
+}
+
 void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 {
 	int i;
@@ -589,39 +626,4 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	wfx_skb_dtor(wvif->wdev, skb);
 }
 
-static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb)
-{
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	struct ieee80211_sta *sta;
-	struct wfx_sta_priv *sta_priv;
-	int tid = ieee80211_get_tid(hdr);
 
-	rcu_read_lock(); // protect sta
-	sta = ieee80211_find_sta(wvif->vif, hdr->addr1);
-	if (sta) {
-		sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
-		spin_lock_bh(&sta_priv->lock);
-		WARN(!sta_priv->buffered[tid], "inconsistent notification");
-		sta_priv->buffered[tid]--;
-		if (!sta_priv->buffered[tid])
-			ieee80211_sta_set_buffered(sta, tid, false);
-		spin_unlock_bh(&sta_priv->lock);
-	}
-	rcu_read_unlock();
-}
-
-void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
-{
-	struct hif_msg *hif = (struct hif_msg *)skb->data;
-	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
-	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
-	unsigned int offset = sizeof(struct hif_req_tx) +
-				sizeof(struct hif_msg) +
-				req->data_flags.fc_offset;
-
-	WARN_ON(!wvif);
-	skb_pull(skb, offset);
-	wfx_notify_buffered_tx(wvif, skb);
-	wfx_tx_policy_put(wvif, req->tx_flags.retry_policy_index);
-	ieee80211_tx_status_irqsafe(wdev->hw, skb);
-}
-- 
2.25.1

[PATCH 28/32] staging: wfx: repair wfx_flush()

[Jerome Pouiller]

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

Until now, wfx_flush() flushed queue for while device instead of only
the queue of the intended vif. It sometime failed with a timeout, but
this error was not reported.

Moreover, if the device was frozen, wfx_flush didn't do anything and it
results a potential warning (and maybe a resource leak) when the frozen
device was unregistered.

We can also notice that wfx_tx_queues_wait_empty_vif() did only exist to
work around the broken feature of wfx_flush().

This patch repair wfx_flush() and therefore drop
wfx_tx_queues_wait_empty_vif().

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c |  34 ++++++-
 drivers/staging/wfx/data_tx.h |   3 +-
 drivers/staging/wfx/main.c    |   1 -
 drivers/staging/wfx/queue.c   | 163 ++++++++++++++++------------------
 drivers/staging/wfx/queue.h   |  10 ++-
 drivers/staging/wfx/sta.c     |  36 +-------
 drivers/staging/wfx/sta.h     |   2 -
 7 files changed, 120 insertions(+), 129 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index ec95518c9167..1d9a8089f3d3 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -524,7 +524,7 @@ static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb)
 	rcu_read_unlock();
 }
 
-void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
+static void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
 {
 	struct hif_msg *hif = (struct hif_msg *)skb->data;
 	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
@@ -626,4 +626,36 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	wfx_skb_dtor(wvif->wdev, skb);
 }
 
+void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+	       u32 queues, bool drop)
+{
+	struct wfx_dev *wdev = hw->priv;
+	struct sk_buff_head dropped;
+	struct wfx_queue *queue;
+	struct sk_buff *skb;
+	int vif_id = -1;
+	int i;
+
+	if (vif)
+		vif_id = ((struct wfx_vif *)vif->drv_priv)->id;
+	skb_queue_head_init(&dropped);
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		if (!(BIT(i) & queues))
+			continue;
+		queue = &wdev->tx_queue[i];
+		if (drop)
+			wfx_tx_queue_drop(wdev, queue, vif_id, &dropped);
+		if (wdev->chip_frozen)
+			continue;
+		if (wait_event_timeout(wdev->tx_dequeue,
+				       wfx_tx_queue_empty(wdev, queue, vif_id),
+				       msecs_to_jiffies(1000)) <= 0)
+			dev_warn(wdev->dev, "frames queued while flushing tx queues?");
+	}
+	wfx_tx_flush(wdev);
+	if (wdev->chip_frozen)
+		wfx_pending_drop(wdev, &dropped);
+	while ((skb = skb_dequeue(&dropped)) != NULL)
+		wfx_skb_dtor(wdev, skb);
+}
 
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index 03fe3e319ba1..7f201f626410 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -44,7 +44,8 @@ void wfx_tx_policy_upload_work(struct work_struct *work);
 void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	    struct sk_buff *skb);
 void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg);
-void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb);
+void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+	       u32 queues, bool drop);
 
 static inline struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb)
 {
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 5e1a7a932b53..738016d45d63 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -267,7 +267,6 @@ static void wfx_free_common(void *data)
 
 	mutex_destroy(&wdev->rx_stats_lock);
 	mutex_destroy(&wdev->conf_mutex);
-	wfx_tx_queues_deinit(wdev);
 	ieee80211_free_hw(wdev->hw);
 }
 
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index a1a2f7756a27..d4302a30dc41 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -62,92 +62,79 @@ void wfx_tx_lock_flush(struct wfx_dev *wdev)
 	wfx_tx_flush(wdev);
 }
 
-/* If successful, LOCKS the TX queue! */
-void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif)
-{
-	int i;
-	bool done;
-	struct wfx_queue *queue;
-	struct sk_buff *item;
-	struct wfx_dev *wdev = wvif->wdev;
-	struct hif_msg *hif;
-
-	if (wvif->wdev->chip_frozen) {
-		wfx_tx_lock_flush(wdev);
-		wfx_tx_queues_clear(wdev);
-		return;
-	}
-
-	do {
-		done = true;
-		wfx_tx_lock_flush(wdev);
-		for (i = 0; i < IEEE80211_NUM_ACS && done; ++i) {
-			queue = &wdev->tx_queue[i];
-			spin_lock_bh(&queue->normal.lock);
-			skb_queue_walk(&queue->normal, item) {
-				hif = (struct hif_msg *)item->data;
-				if (hif->interface == wvif->id)
-					done = false;
-			}
-			spin_unlock_bh(&queue->normal.lock);
-			spin_lock_bh(&queue->cab.lock);
-			skb_queue_walk(&queue->cab, item) {
-				hif = (struct hif_msg *)item->data;
-				if (hif->interface == wvif->id)
-					done = false;
-			}
-			spin_unlock_bh(&queue->cab.lock);
-		}
-		if (!done) {
-			wfx_tx_unlock(wdev);
-			msleep(20);
-		}
-	} while (!done);
-}
-
-static void wfx_tx_queue_clear(struct wfx_dev *wdev, struct wfx_queue *queue,
-			       struct sk_buff_head *gc_list)
-{
-	struct sk_buff *item;
-
-	while ((item = skb_dequeue(&queue->normal)) != NULL)
-		skb_queue_head(gc_list, item);
-	while ((item = skb_dequeue(&queue->cab)) != NULL)
-		skb_queue_head(gc_list, item);
-}
-
-void wfx_tx_queues_clear(struct wfx_dev *wdev)
-{
-	int i;
-	struct sk_buff *item;
-	struct sk_buff_head gc_list;
-
-	skb_queue_head_init(&gc_list);
-	for (i = 0; i < IEEE80211_NUM_ACS; ++i)
-		wfx_tx_queue_clear(wdev, &wdev->tx_queue[i], &gc_list);
-	wake_up(&wdev->tx_dequeue);
-	while ((item = skb_dequeue(&gc_list)) != NULL)
-		wfx_skb_dtor(wdev, item);
-}
-
 void wfx_tx_queues_init(struct wfx_dev *wdev)
 {
 	int i;
 
-	memset(wdev->tx_queue, 0, sizeof(wdev->tx_queue));
 	skb_queue_head_init(&wdev->tx_pending);
 	init_waitqueue_head(&wdev->tx_dequeue);
-
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
 		skb_queue_head_init(&wdev->tx_queue[i].normal);
 		skb_queue_head_init(&wdev->tx_queue[i].cab);
 	}
 }
 
-void wfx_tx_queues_deinit(struct wfx_dev *wdev)
+void wfx_tx_queues_check_empty(struct wfx_dev *wdev)
 {
-	WARN_ON(!skb_queue_empty(&wdev->tx_pending));
-	wfx_tx_queues_clear(wdev);
+	int i;
+
+	WARN_ON(!skb_queue_empty_lockless(&wdev->tx_pending));
+	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
+		WARN_ON(atomic_read(&wdev->tx_queue[i].pending_frames));
+		WARN_ON(!skb_queue_empty_lockless(&wdev->tx_queue[i].normal));
+		WARN_ON(!skb_queue_empty_lockless(&wdev->tx_queue[i].cab));
+	}
+}
+
+static bool __wfx_tx_queue_empty(struct wfx_dev *wdev,
+				 struct sk_buff_head *skb_queue, int vif_id)
+{
+	struct hif_msg *hif_msg;
+	struct sk_buff *skb;
+
+	spin_lock_bh(&skb_queue->lock);
+	skb_queue_walk(skb_queue, skb) {
+		hif_msg = (struct hif_msg *)skb->data;
+		if (vif_id < 0 || hif_msg->interface == vif_id) {
+			spin_unlock_bh(&skb_queue->lock);
+			return false;
+		}
+	}
+	spin_unlock_bh(&skb_queue->lock);
+	return true;
+}
+
+bool wfx_tx_queue_empty(struct wfx_dev *wdev,
+			struct wfx_queue *queue, int vif_id)
+{
+	return __wfx_tx_queue_empty(wdev, &queue->normal, vif_id) &&
+	       __wfx_tx_queue_empty(wdev, &queue->cab, vif_id);
+}
+
+static void __wfx_tx_queue_drop(struct wfx_dev *wdev,
+				struct sk_buff_head *skb_queue, int vif_id,
+				struct sk_buff_head *dropped)
+{
+	struct sk_buff *skb, *tmp;
+	struct hif_msg *hif_msg;
+
+	spin_lock_bh(&skb_queue->lock);
+	skb_queue_walk_safe(skb_queue, skb, tmp) {
+		hif_msg = (struct hif_msg *)skb->data;
+		if (vif_id < 0 || hif_msg->interface == vif_id) {
+			__skb_unlink(skb, skb_queue);
+			skb_queue_head(dropped, skb);
+		}
+	}
+	spin_unlock_bh(&skb_queue->lock);
+}
+
+void wfx_tx_queue_drop(struct wfx_dev *wdev, struct wfx_queue *queue,
+		       int vif_id, struct sk_buff_head *dropped)
+{
+	__wfx_tx_queue_drop(wdev, &queue->cab, vif_id, dropped);
+	__wfx_tx_queue_drop(wdev, &queue->normal, vif_id, dropped);
+	wake_up(&wdev->tx_dequeue);
 }
 
 void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb)
@@ -174,6 +161,22 @@ int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb)
 	return 0;
 }
 
+void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped)
+{
+	struct wfx_queue *queue;
+	struct sk_buff *skb;
+
+	WARN(!wdev->chip_frozen, "%s should only be used to recover a frozen device",
+	     __func__);
+	while ((skb = skb_dequeue(&wdev->tx_pending)) != NULL) {
+		queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
+		WARN_ON(skb_get_queue_mapping(skb) > 3);
+		WARN_ON(!atomic_read(&queue->pending_frames));
+		atomic_dec(&queue->pending_frames);
+		skb_queue_head(dropped, skb);
+	}
+}
+
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
 {
 	struct wfx_queue *queue;
@@ -249,17 +252,6 @@ bool wfx_tx_queues_has_cab(struct wfx_vif *wvif)
 	return false;
 }
 
-bool wfx_tx_queues_empty(struct wfx_dev *wdev)
-{
-	int i;
-
-	for (i = 0; i < IEEE80211_NUM_ACS; i++)
-		if (!skb_queue_empty_lockless(&wdev->tx_queue[i].normal) ||
-		    !skb_queue_empty_lockless(&wdev->tx_queue[i].cab))
-			return false;
-	return true;
-}
-
 static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
 {
 	struct hif_req_tx *req = wfx_skb_txreq(skb);
@@ -364,8 +356,7 @@ struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev)
 		if (!skb)
 			return NULL;
 		skb_queue_tail(&wdev->tx_pending, skb);
-		if (wfx_tx_queues_empty(wdev))
-			wake_up(&wdev->tx_dequeue);
+		wake_up(&wdev->tx_dequeue);
 		// FIXME: is it useful?
 		if (wfx_handle_tx_data(wdev, skb))
 			continue;
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 9bc1a5200e64..ab45e32cbfbc 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -31,16 +31,18 @@ void wfx_tx_flush(struct wfx_dev *wdev);
 void wfx_tx_lock_flush(struct wfx_dev *wdev);
 
 void wfx_tx_queues_init(struct wfx_dev *wdev);
-void wfx_tx_queues_deinit(struct wfx_dev *wdev);
-void wfx_tx_queues_clear(struct wfx_dev *wdev);
-bool wfx_tx_queues_empty(struct wfx_dev *wdev);
+void wfx_tx_queues_check_empty(struct wfx_dev *wdev);
 bool wfx_tx_queues_has_cab(struct wfx_vif *wvif);
-void wfx_tx_queues_wait_empty_vif(struct wfx_vif *wvif);
 void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb);
 struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev);
 
+bool wfx_tx_queue_empty(struct wfx_dev *wdev, struct wfx_queue *queue,
+			int vif_id);
+void wfx_tx_queue_drop(struct wfx_dev *wdev, struct wfx_queue *queue,
+		       int vif_id, struct sk_buff_head *dropped);
 
 struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
+void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped);
 int wfx_pending_requeue(struct wfx_dev *wdev, struct sk_buff *skb);
 unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 					  struct sk_buff *skb);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 340e09bb639d..b1ee02d2f515 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -318,29 +318,6 @@ int wfx_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	return 0;
 }
 
-static int __wfx_flush(struct wfx_dev *wdev, bool drop)
-{
-	for (;;) {
-		if (drop)
-			wfx_tx_queues_clear(wdev);
-		if (wait_event_timeout(wdev->tx_dequeue,
-				       wfx_tx_queues_empty(wdev),
-				       2 * HZ) <= 0)
-			return -ETIMEDOUT;
-		wfx_tx_flush(wdev);
-		if (wfx_tx_queues_empty(wdev))
-			return 0;
-		dev_warn(wdev->dev, "frames queued while flushing tx queues");
-	}
-}
-
-void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
-		  u32 queues, bool drop)
-{
-	// FIXME: only flush requested vif and queues
-	__wfx_flush(hw->priv, drop);
-}
-
 /* WSM callbacks */
 
 static void wfx_event_report_rssi(struct wfx_vif *wvif, u8 raw_rcpi_rssi)
@@ -843,10 +820,8 @@ static int wfx_update_tim(struct wfx_vif *wvif)
 
 	skb = ieee80211_beacon_get_tim(wvif->wdev->hw, wvif->vif,
 				       &tim_offset, &tim_length);
-	if (!skb) {
-		__wfx_flush(wvif->wdev, true);
+	if (!skb)
 		return -ENOENT;
-	}
 	tim_ptr = skb->data + tim_offset;
 
 	if (tim_offset && tim_length >= 6) {
@@ -1062,8 +1037,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 	}
 
 	wvif->state = WFX_STATE_PASSIVE;
-	wfx_tx_queues_wait_empty_vif(wvif);
-	wfx_tx_unlock(wdev);
 
 	/* FIXME: In add to reset MAC address, try to reset interface */
 	hif_set_macaddr(wvif, NULL);
@@ -1097,10 +1070,5 @@ void wfx_stop(struct ieee80211_hw *hw)
 {
 	struct wfx_dev *wdev = hw->priv;
 
-	wfx_tx_lock_flush(wdev);
-	mutex_lock(&wdev->conf_mutex);
-	wfx_tx_queues_clear(wdev);
-	mutex_unlock(&wdev->conf_mutex);
-	wfx_tx_unlock(wdev);
-	WARN(atomic_read(&wdev->tx_lock), "tx_lock is locked");
+	wfx_tx_queues_check_empty(wdev);
 }
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index cf99a8a74a81..a0c5153e5272 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -54,8 +54,6 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 
 int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 void wfx_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
-void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
-	       u32 queues, bool drop);
 int wfx_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		u16 queue, const struct ieee80211_tx_queue_params *params);
 void wfx_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
-- 
2.25.1

[PATCH 29/32] staging: wfx: wfx_flush() did not ensure that frames are processed

[Jerome Pouiller]

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

wfx_flush() exited once all frames are retrieved from the device.
However, it did not ensure they were processed by driver before to
return. Therefore, some frame may be processed after the interface has
disappear.

Change the place we signal that the queue is empty to fix that.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/bh.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 9fcab00a3733..ba7fa0a7cd9a 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -108,8 +108,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 			release_count = 1;
 		WARN(wdev->hif.tx_buffers_used < release_count, "corrupted buffer counter");
 		wdev->hif.tx_buffers_used -= release_count;
-		if (!wdev->hif.tx_buffers_used)
-			wake_up(&wdev->hif.tx_buffers_empty);
 	}
 	_trace_hif_recv(hif, wdev->hif.tx_buffers_used);
 
@@ -123,6 +121,8 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	skb_put(skb, hif->len);
 	// wfx_handle_rx takes care on SKB livetime
 	wfx_handle_rx(wdev, skb);
+	if (!wdev->hif.tx_buffers_used)
+		wake_up(&wdev->hif.tx_buffers_empty);
 
 	return piggyback;
 
-- 
2.25.1

[PATCH 30/32] staging: wfx: fix potential deadlock in wfx_tx_flush()

[Jerome Pouiller]

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

wfx_tx_flush() wait there are no more frame in device buffer. However,
this event may never happens since wfx_tx_flush() don't forbid to
enqueue new frames.

Note that wfx_tx_flush() should only ensure that all frames currently in
hardware queues are sent. So the current code is more restrictive that
it should.

Note that wfx_tx_flush() release the lock before to return while
wfx_tx_lock_flush() keep the lock.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index d4302a30dc41..e6d7d0e45156 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -36,6 +36,7 @@ void wfx_tx_flush(struct wfx_dev *wdev)
 	if (wdev->chip_frozen)
 		return;
 
+	wfx_tx_lock(wdev);
 	mutex_lock(&wdev->hif_cmd.lock);
 	ret = wait_event_timeout(wdev->hif.tx_buffers_empty,
 				 !wdev->hif.tx_buffers_used,
@@ -54,6 +55,7 @@ void wfx_tx_flush(struct wfx_dev *wdev)
 		wdev->chip_frozen = 1;
 	}
 	mutex_unlock(&wdev->hif_cmd.lock);
+	wfx_tx_unlock(wdev);
 }
 
 void wfx_tx_lock_flush(struct wfx_dev *wdev)
-- 
2.25.1

[PATCH 31/32] staging: wfx: fix case where AP stop with CAB traffic pending

[Jerome Pouiller]

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

When driver has Content After DTIM Beacon (CAB) in queue, it wait for an
indication from the firmware. However, when we stop to send beacons,
this indication may never happen.

Solve this issue by simply simulate this indication. Firmware will send
data that probably nobody will heard.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index b1ee02d2f515..2e8d3f571c3e 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -686,6 +686,19 @@ static void wfx_join_finalize(struct wfx_vif *wvif,
 	}
 }
 
+void wfx_enable_beacon(struct wfx_vif *wvif, bool enable)
+{
+	// Driver has Content After DTIM Beacon in queue. Driver is waiting for
+	// a signal from the firmware. Since we are going to stop to send
+	// beacons, this signal will never happens. See also
+	// wfx_suspend_resume_mc()
+	if (!enable && wfx_tx_queues_has_cab(wvif)) {
+		wvif->after_dtim_tx_allowed = true;
+		wfx_bh_request_tx(wvif->wdev);
+	}
+	hif_beacon_transmit(wvif, enable);
+}
+
 void wfx_bss_info_changed(struct ieee80211_hw *hw,
 			     struct ieee80211_vif *vif,
 			     struct ieee80211_bss_conf *info,
@@ -724,7 +737,7 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw,
 
 	if (changed & BSS_CHANGED_BEACON_ENABLED &&
 	    wvif->state != WFX_STATE_IBSS)
-		hif_beacon_transmit(wvif, info->enable_beacon);
+		wfx_enable_beacon(wvif, info->enable_beacon);
 
 	if (changed & BSS_CHANGED_BEACON_INFO)
 		hif_set_beacon_wakeup_period(wvif, info->dtim_period,
-- 
2.25.1

[PATCH 32/32] staging: wfx: remove hack about tx_rate policies

[Jerome Pouiller]

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

Current code contains a weird hack to avoid switch from 54Mbps CTS to
1Mbps. However, we have not been able to reproduce the problem and
hardware team don't know any defect of this kind. So, it seems this hack
is no more necessary.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 53 -----------------------------------
 1 file changed, 53 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 1d9a8089f3d3..93ed0ed63bb2 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -51,59 +51,6 @@ static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
 		if (rates[i].idx < 0)
 			break;
 	count = i;
-
-	/* HACK!!! Device has problems (at least) switching from
-	 * 54Mbps CTS to 1Mbps. This switch takes enormous amount
-	 * of time (100-200 ms), leading to valuable throughput drop.
-	 * As a workaround, additional g-rates are injected to the
-	 * policy.
-	 */
-	if (count == 2 && !(rates[0].flags & IEEE80211_TX_RC_MCS) &&
-	    rates[0].idx > 4 && rates[0].count > 2 &&
-	    rates[1].idx < 2) {
-		int mid_rate = (rates[0].idx + 4) >> 1;
-
-		/* Decrease number of retries for the initial rate */
-		rates[0].count -= 2;
-
-		if (mid_rate != 4) {
-			/* Keep fallback rate at 1Mbps. */
-			rates[3] = rates[1];
-
-			/* Inject 1 transmission on lowest g-rate */
-			rates[2].idx = 4;
-			rates[2].count = 1;
-			rates[2].flags = rates[1].flags;
-
-			/* Inject 1 transmission on mid-rate */
-			rates[1].idx = mid_rate;
-			rates[1].count = 1;
-
-			/* Fallback to 1 Mbps is a really bad thing,
-			 * so let's try to increase probability of
-			 * successful transmission on the lowest g rate
-			 * even more
-			 */
-			if (rates[0].count >= 3) {
-				--rates[0].count;
-				++rates[2].count;
-			}
-
-			/* Adjust amount of rates defined */
-			count += 2;
-		} else {
-			/* Keep fallback rate at 1Mbps. */
-			rates[2] = rates[1];
-
-			/* Inject 2 transmissions on lowest g-rate */
-			rates[1].idx = 4;
-			rates[1].count = 2;
-
-			/* Adjust amount of rates defined */
-			count += 1;
-		}
-	}
-
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; ++i) {
 		int rateid;
 		u8 count;
-- 
2.25.1

Re: [PATCH 00/32] staging: wfx: rework the Tx queue

[Dan Carpenter]

I didn't quite finish reviewing these pathches last night.  Looks good.
You will need a check on "ssidlen" to prevent memory corruption, as
discussed in patch 1, but that's not a bug which was introduced by this
patchset.  None of my other comments really applied to the patchset
itself, just to the surrounding code.

Looks good.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter