Re: [PATCH v2 05/22] brcmsmac: Use IEEE 802.11 AC levels for pktq precedence levels

["Arend van Spriel" <arend@broadcom.com>]

On 11/15/2012 03:07 PM, Seth Forshee wrote:
> The mac80211 tx queues and brcmsmac DMA fifos both map directly to AC
> levels. Therefore it's much more straightforward to queue tx frames and
> choose the tx fifo based on the mac80211 queue instead of mapping 802.1D
> priority tags to precedence levels then back to AC levels. mac80211
> already maps the 802.1D levels to the appropriate AC levels and queues
> management frames at the maximum priority, so the results should be
> identical.
>
> One functional change resulting from this patch is that AMPDU retries no
> longer get a priority boost to queue them ahead of packets with the same

True. I believe the statement actually applies to any retry include 
non-AMPDU.

> priority already in the tx queue. This behavior will be restored (in
> effect at least) in a later patch when the tx queue is removed.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>   drivers/net/wireless/brcm80211/brcmsmac/ampdu.c |   13 +----
>   drivers/net/wireless/brcm80211/brcmsmac/main.c  |   67 ++++++++---------------
>   drivers/net/wireless/brcm80211/brcmsmac/main.h  |    6 +-
>   drivers/net/wireless/brcm80211/brcmsmac/pub.h   |   26 +--------
>   4 files changed, 26 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> index 135af9a..8abf39d 100644
> --- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> @@ -365,6 +342,21 @@ static const char fifo_names[6][0];
>   static struct brcms_c_info *wlc_info_dbg = (struct brcms_c_info *) (NULL);
>   #endif
>
> +/* Mapping of ieee80211 AC numbers to tx fifos */
> +static const u8 ac_to_fifo_mapping[IEEE80211_NUM_ACS] = {
> +	[IEEE80211_AC_VO]	= TX_AC_VO_FIFO,
> +	[IEEE80211_AC_VI]	= TX_AC_VI_FIFO,
> +	[IEEE80211_AC_BE]	= TX_AC_BE_FIFO,
> +	[IEEE80211_AC_BK]	= TX_AC_BK_FIFO,

Minor comment: Can you get rid of the tab before the equal sign and use 
space instead?

> @@ -6068,14 +6054,13 @@ static bool brcms_c_prec_enq(struct brcms_c_info *wlc, struct pktq *q,
>   }
>
>   void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb,
> -		     struct sk_buff *sdu, uint prec)
> +		     struct sk_buff *sdu)
>   {
>   	struct brcms_txq_info *qi = wlc->pkt_queue;	/* Check me */
>   	struct pktq *q = &qi->q;
> -	int prio;
> -
> -	prio = sdu->priority;
> +	uint prec;

the term prec, short for precedence, may be confusing as the concept is 
removed.

> +	prec = brcms_ac_to_fifo(skb_get_queue_mapping(sdu));
>   	if (!brcms_c_prec_enq(wlc, q, sdu, prec)) {
>   		/*
>   		 * we might hit this condtion in case