Re: [PATCH v2] brcmsmac: restructure info->control.sta handling as it is goning to be removed soon.

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

On 07/06/2012 09:37 PM, John W. Linville wrote:
> On Fri, Jun 29, 2012 at 11:04:16AM -0400, thomas wrote:
>> Johannes Berg schrieb:
>>
>>> On Fri, 2012-06-29 at 05:56 -0700, Thomas Huehn wrote:
>>>> brcmsmac uses info->control.sta while doing ampdu aggregation. This patch
>>>> changes the usage of the structure info->control.sta, as it is going to be
>>>> removed soon from struct ieee80211_tx_info. This patch is a pre-requisit in
>>>> order to add transmission power control (TPC) to the mac80211 subsystem.
>>>>
>>>> Suggested-by: Felix Fietkau <nbd@openwrt.org>
>>>> Signed-off-by: Thomas Huehn <thomas@net.t-labs.tu-berlin.de>
>>>> ---
>>>> Change sequence of overwriting data. Thx to Johannes.
>>>
>>>
>>>> @@ -665,7 +665,7 @@ brcms_c_sendampdu(struct ampdu_info *ampdu, struct brcms_txq_info *qi,
>>>>  			u8 plcp0, plcp3, is40, sgi;
>>>>  			struct ieee80211_sta *sta;
>>>>  
>>>> -			sta = tx_info->control.sta;
>>>> +			sta = tx_info->rate_driver_data[0];
>>>
>>> Are you sure it actually works now?
>>
>>
>> From locking at the code, sta is never used after it gets overridden.
>> The brcmsmac driver does not use the vif pointer at all. I asked Felix
>> to double check as I do not have a Broadcom device over here.
>>
>> Bye Thomas
> 
> Any comments on this one from the Broadcom camp?
> 

Hi John,

Just applied the patch to have a closer look at the changes, but I am
missing details on the whole story. I see tx_info->rate_driver_data[0]
being set in the tx callback and reset to NULL in some other path which
I believe is only called upon tid or sta removal so it does not make
sense to me.

I have pushed the patch to our internal review server as I would like
some co-workers to have a look. If anyone can provide any more details
what this patch is trying to solve, please do.

Gr. AvS