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@xxxxxxxxxxx> >>>> Signed-off-by: Thomas Huehn <thomas@xxxxxxxxxxxxxxxxxxxxxxx> >>>> --- >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
