Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support

[Oleksij Rempel <linux@rempel-privat.de>]

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?
-- 
Regards,
Oleksij