Re: [PATCH 2/4] ath6kl: Introduce new queue for uAPSD.

[Kalle Valo <kvalo@qca.qualcomm.com>]

On 01/12/2012 07:25 AM, Pachamuthu, Thirumalai wrote:

>> -----Original Message-----
>> From: Valo, Kalle
>> Sent: Wednesday, January 11, 2012 5:58 PM
>> To: Pachamuthu, Thirumalai
>> Cc: linux-wireless@vger.kernel.org; ath6kl-devel
>> Subject: Re: [PATCH 2/4] ath6kl: Introduce new queue for uAPSD.
>>
>> On 01/09/2012 09:45 AM, Thirumalai wrote:
>>> Add apsd_info in necessary places to indicate which AC's uAPSD is
>>> enabled.
>>>
>>> A bit is added in the wmi header (bit 4 of info3) to specify uapsd
>>> trigger in rx direction (target to host) and end of service period in
>>> tx direction (host to target)
>>>
>>> Signed-off-by: Thirumalai <tpachamu@qca.qualcomm.com>
>>
>> [...]
>>
>>> +#define WMI_DATA_HDR_TRIGGER_MASK      0x1
>>> +#define WMI_DATA_HDR_TRIGGER_SHIFT     4
>>> +
>>> +#define WMI_DATA_HDR_EOSP_MASK      WMI_DATA_HDR_TRIGGER_MASK
>>> +#define WMI_DATA_HDR_EOSP_SHIFT     WMI_DATA_HDR_TRIGGER_SHIFT
>>
>> This looks a bit odd, I can't see why you want to use trigger definitions for eosp.
>> Even if the values are same I don't see the benefit for using trigger values for
>> eosp. Is there a particular reason for this?
>>
>> Kalle
> 
> The bit in the wmi header (bit 4 of info3) to specify TRIGGER in RX direction (target to host) and the same bit will be used as ESOP in TX direction (host to target). 
> Simply we reused the existing macro which made this confusion; I will make the following change in the next release.
> 
> #define WMI_DATA_HDR_EOSP_MASK    1
> #define WMI_DATA_HDR_EOSP_SHIFT    4

Thanks, please do that. It is definitely clearer that way.

Kalle