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

From: Dan Carpenter
Date: Thu Apr 02 2020 - 09:07:47 EST


On Wed, Apr 01, 2020 at 01:03:37PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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