| Message ID | 20200401110405.80282-5-Jerome.Pouiller@silabs.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | staging: wfx: rework the Tx queue | expand |
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
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.
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;