Am 04.05.2013 13:16, schrieb Felix Fietkau: > On 2013-05-04 1:08 PM, Oleksij Rempel wrote: >> Am 04.05.2013 12:02, schrieb Felix Fietkau: >>> On 2013-05-04 8:50 AM, Oleksij Rempel wrote: >>>> Am 02.05.2013 22:15, schrieb Adrian Chadd: >>>>> Well, let's dig into the firmware a bit more and tidy up how STBC is >>>>> handled. >>>> >>>> Does it mean, i should change this patch and provide a patch for >>>> firmware too? >>>> I still do not think, changing peer caps i a good idea in any case. >>>> I mena this part of patch: >>>> + if (sta->ht_cap.cap & IEEE80211_HT_CAP_TX_STBC) >>>> + caps |= WLAN_RC_TX_STBC_FLAG; >>>> >>>> >>>> Behaviour with this patch will be fallowing: >>>> - peer provide caps, even if it is RX-STBC12 >>>> - we pass this information to firmware and ratecontroller of FW, do >>>> right decision based on hardware it has. >>>> >>>> You suggestion, if i understand it correctly, is to filter caps: >>>> - if peer provide more than we can, we should tell firmware the value >>>> what we can. So, if peer say it can RX-STBC12, we should tell firmware >>>> that peer is RX-STBC1. >>>> In my opinion, this kind of filter is a source for hidden errors. >>> I think the discussion regarding RX-STBC12 vs RX-STBC1 is purely >>> hypothetical. The hardware that this firmware was designed for only >>> supports sending STBC for MCS0-7. This will not change. >>> >>> Also, there's no reason to tell the firmware about both rx and tx STBC >>> capabilities, the only thing it needs to know is what tells the rate >>> control part to enable/disable STBC. >> >> As you can see, in version 2 of this path there was no more discussion >> about it. I just did it. >> >>> In addition to that, using the WLAN_RC_* flags is wrong, you need to use >>> the ATH_RC_* flags, as this is what ath_rate_newassoc_11n checks for in >>> the firmware. >> >> Renamed. >> >>> The only STBC related flag that actually gets used (when >>> passed from the driver) is ATH_RC_RX_STBC_FLAG. >> >> Well, may be it is not used at the end. But it is present on some places >> in the firmware. >> For example here: >> void >> rcSibUpdate_11n(struct ath_softc_tgt *sc, struct ath_node_target *pSib, >> A_UINT32 capflag, A_BOOL keepState, struct >> ieee80211_rate *pRateSet) >> { >> rcSibUpdate_ht(sc, >> pSib, >> ((capflag & ATH_RC_DS_FLAG) ? WLAN_RC_DS_FLAG >> : 0) | >> ((capflag & ATH_RC_HT40_SGI_FLAG) ? >> WLAN_RC_HT40_SGI_FLAG : 0) | >> ((capflag & ATH_RC_HT_FLAG) ? WLAN_RC_HT_FLAG >> : 0) | >> ((capflag & ATH_RC_CW40_FLAG) ? WLAN_RC_40_FLAG >> : 0) | >> ((capflag & ATH_RC_TX_STBC_FLAG) ? >> WLAN_RC_STBC_FLAG : 0), >> keepState, >> pRateSet); >> >> >> >> So, should i remove ATH_RC_TX_STBC_FLAG from my patch? > I extensively reviewed this part, and it's really crazy. Here's what > happens: > > ath_rate_newassoc_11n takes ATH_RC_* flags, converts them to WLAN_RC_*. > rcSibUpdate_11n interprets the WLAN_RC_* flags as ATH_RC_* and converts > them to WLAN_RC_* again. For most flags this is pretty much a no-op > because the definitions are identical. > For STBC the result 'accidentally' still contains WLAN_RC_STBC_FLAG, but > only because ath_rate_newassoc_11n converts ATH_RC_RX_STBC_FLAG to > WLAN_RC_STBC_FLAG and WLAN_RC_STBC_FLAG overlaps with ATH_RC_TX_STBC_FLAG. > In the end it doesn't matter anymore, because nothing in the code takes > the STBC info from the capflags. > > STBC is used if ATH_NODE_ATHEROS(an)->stbc is non-zero, and this gets > set by ath_rate_newassoc_11n before all of those incredibly moronic > conversions happen.
Ok, thx. I'll remove it from my patch. And will remove it from firmware. Even if you wont to remove bigger part of firmware, i thing it wont happen this year? -- Regards, Oleksij _______________________________________________ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel