[ath9k-devel] [PATCH 3/3] ath10k: enable STA UAPSD support

[Michal Kazior <michal.kazior@tieto.com>]

On 09/05/13 09:47, Janusz.Dziedzic at tieto.com wrote:
>> -----Original Message-----
>> From: Kalle Valo [mailto:kvalo at qca.qualcomm.com]
>> Sent: 9 maja 2013 09:17
>> To: Dziedzic Janusz
>> Cc: ath9k-devel at lists.ath9k.org
>> Subject: Re: [ath9k-devel] [PATCH 3/3] ath10k: enable STA UAPSD support
>>
>> Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:
>>
>>> Enable UAPSD support for STA mode.
>>>
>>> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
>>
>> Same here, please explain briefly what you have tested.
>>
>>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>>> @@ -2296,6 +2296,68 @@ static int ath10k_sta_state(struct ieee80211_hw
>> *hw,
>>>   	return ret;
>>>   }
>>>
>>> +#define SET_UAPSD(enable, out, flags) do {	\
>>> +	if ((enable))				\
>>> +		(out) |= (flags);		\
>>> +	else					\
>>> +		(out) &= ~(flags);		\
>>> +} while (0)
>>
>> Why the macro? To workaround a long line warning from checkpatch?
>>
>> If that's a problem we could increate line length limit, for example to
>> 85 or 90. In some cases the 80 char limit is a bit too excessive. Would that
>> help?
>
> Not really. This is because we have to set/clean this flags for every ACs.
> So, this is just to prevent code duplication.
>
>          case IEEE80211_AC_VO:
>                 if (enable)
>                          ...
>                 else
>                        ....
>                 break;
>         case IEEE80211_AC_VI:
>                 if (enable)
>                        ....
>                  else
>                       ....
>                 break;
>          .....
>
> I can remove this macro and create inline function instead.

Hmm, but since this is per-ac can't we do the following?

switch (ac) {
case IEEE80211_AC_VO:
	val = WMI_STA_PS_UAPSD_AC3_DELIVERY_EN |
               WMI_STA_PS_UAPSD_AC3_TRIGGER_EN;
	break;
// ..
}

if (enable)
	arvif->u.sta.uapsd |= val;
else
	arvif->u.sta.uapsd &= ~val;


No macros and no code duplication.


-- Pozdrawiam / Best regards, Michal Kazior.