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
--
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
