[ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
current firmware will enable STBC_TX, only if other peer support it.
This patch provide ht_peer_caps to firmware.
FW versions 1.3, 1.4 should be able to work with it.
Tested on ar7010+ar9280 and ar7010+ar9287.
Signed-off-by: Oleksij Rempel
---
drivers/net/wireless/ath/ath9k/htc.h | 2 ++
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 3 +++
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 4
3 files changed, 9 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/htc.h
b/drivers/net/wireless/ath/ath9k/htc.h
index d3b099d..db4a793 100644
--- a/drivers/net/wireless/ath/ath9k/htc.h
+++ b/drivers/net/wireless/ath/ath9k/htc.h
@@ -142,6 +142,8 @@ struct ath9k_htc_target_aggr {
#define WLAN_RC_40_FLAG 0x02
#define WLAN_RC_SGI_FLAG 0x04
#define WLAN_RC_HT_FLAG 0x08
+#define WLAN_RC_TX_STBC_FLAG 0x20 /* TX STBC */
+#define WLAN_RC_RX_STBC_FLAG 0xC0 /* RX STBC ,2 bits */
struct ath9k_htc_rateset {
u8 rs_nrates;
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index a47f5e0..c79c5ac 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -517,6 +517,9 @@ static void setup_ht_cap(struct ath9k_htc_priv *priv,
ath_dbg(common, CONFIG, "TX streams %d, RX streams: %d\n",
tx_streams, rx_streams);
+ if (tx_streams >= 2)
+ ht_info->cap |= IEEE80211_HT_CAP_TX_STBC;
+
if (tx_streams != rx_streams) {
ht_info->mcs.tx_params |= IEEE80211_HT_MCS_TX_RX_DIFF;
ht_info->mcs.tx_params |= ((tx_streams - 1) <<
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index 0743a47..af08b4a 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -623,6 +623,10 @@ static void ath9k_htc_setup_rate(struct ath9k_htc_priv
*priv,
trate->rates.ht_rates.rs_nrates = j;
caps = WLAN_RC_HT_FLAG;
+ if (sta->ht_cap.cap & IEEE80211_HT_CAP_RX_STBC)
+ caps |= WLAN_RC_RX_STBC_FLAG;
+ if (sta->ht_cap.cap & IEEE80211_HT_CAP_TX_STBC)
+ caps |= WLAN_RC_TX_STBC_FLAG;
if (sta->ht_cap.mcs.rx_mask[1])
caps |= WLAN_RC_DS_FLAG;
if ((sta->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
--
1.8.1.2
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
On 2 May 2013 01:11, Oleksij Rempel wrote: > +#define WLAN_RC_TX_STBC_FLAG 0x20 /* TX STBC */ > +#define WLAN_RC_RX_STBC_FLAG 0xC0 /* RX STBC ,2 bits */ I thought we covered this; why are you marking two bits here? Atheros 11n hardware only supports 1-stream STBC RX. Have you verified that we're actually negotiating 1-stream STBC RX with a peer? (Ie, by looking at packet captures?) Adrian ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
Am 02.05.2013 18:55, schrieb Adrian Chadd: > On 2 May 2013 01:11, Oleksij Rempel wrote: > >> +#define WLAN_RC_TX_STBC_FLAG 0x20 /* TX STBC */ >> +#define WLAN_RC_RX_STBC_FLAG 0xC0 /* RX STBC ,2 bits */ > > I thought we covered this; why are you marking two bits here? becouse firmware checks for two bits (and then use it as bool ;)), so i pass what firmware can handle. > Atheros 11n hardware only supports 1-stream STBC RX. Did you got my email with lots of assumptions and questions? What do you mean by 1-stream STBC RX? After i did some home work on STBC i see that it encoded from at least two spatial streams. Is 1-stream STBC RX = 2 spatial streams with mirrored data? and 2-stream STBC RX = 4 spatial streams with mirrored data? or 1-stream STBC RX = compatibility mode for one stream hardware(so only of two streams received)? That would make sense for 1x1:1 hardware, but if you say all atheros N hardware support only 1-stream STBC RX, will mean that STBC is useless on this hardware. > Have you verified that we're actually negotiating 1-stream STBC RX > with a peer? (Ie, by looking at packet captures?) Yes, i wrote this too :) I verified it, we negotiate only 1-stream STBC RX. -- Regards, Oleksij ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
On 2013-05-02 7:32 PM, Oleksij Rempel wrote: > Am 02.05.2013 18:55, schrieb Adrian Chadd: >> On 2 May 2013 01:11, Oleksij Rempel wrote: >> >>> +#define WLAN_RC_TX_STBC_FLAG 0x20 /* TX STBC */ >>> +#define WLAN_RC_RX_STBC_FLAG 0xC0 /* RX STBC ,2 bits */ >> >> I thought we covered this; why are you marking two bits here? > > becouse firmware checks for two bits (and then use it as bool ;)), so i > pass what firmware can handle. > >> Atheros 11n hardware only supports 1-stream STBC RX. > > Did you got my email with lots of assumptions and questions? > What do you mean by 1-stream STBC RX? After i did some home work on STBC > i see that it encoded from at least two spatial streams. > Is > 1-stream STBC RX = 2 spatial streams with mirrored data? > and > 2-stream STBC RX = 4 spatial streams with mirrored data? > > or > > 1-stream STBC RX = compatibility mode for one stream hardware(so only of > two streams received)? When you're talking about 'streams', please specify where you're talking about Spatial Streams (Nss, defined by the MCS), or Space-Time Streams (Nsts). STBC is useful whenever the number of possible Space-Time streams exceeds the number of Spatial streams, i.e. if the number of tx chains is bigger than the number of spatial streams. There's an asymmetry between Rx and Tx here. If a receiver has 1 chain and the transmitter has 2 chains, tx can use 2 Space-Time streams to encode 1 Spatial stream to improve the reliability of the signal. The HT STBC capability field indicates the maximum number of Spatial Streams, not Space-Time streams. Atheros hardware only supports STBC with Nss = 1, so announcing 2-stream STBC is definitely wrong. > That would make sense for 1x1:1 hardware, but if you say all atheros N > hardware support only 1-stream STBC RX, will mean that STBC is useless > on this hardware. Only STBC With Nss=1, Nsts=2 is supported, but this does not make it useless at all. It helps, even if the receiver only has one antenna. - Felix ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
Well, let's dig into the firmware a bit more and tidy up how STBC is handled. Adrian ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
Hi Felix, thank you for your explanation and being pation with me. I learn it by my self and keywords you gave help me to find needed information. So, i continue to digg in to google books and wikis now. I see now, that my initial assumption that STBC some thing like "frequency diversity" is wrong. Well, it say by itself Space and Time, no freq :) Am 02.05.2013 20:01, schrieb Felix Fietkau: > On 2013-05-02 7:32 PM, Oleksij Rempel wrote: >> Am 02.05.2013 18:55, schrieb Adrian Chadd: >>> On 2 May 2013 01:11, Oleksij Rempel wrote: >>> +#define WLAN_RC_TX_STBC_FLAG 0x20 /* TX STBC */ +#define WLAN_RC_RX_STBC_FLAG 0xC0 /* RX STBC ,2 bits */ >>> >>> I thought we covered this; why are you marking two bits here? >> >> becouse firmware checks for two bits (and then use it as bool ;)), so i >> pass what firmware can handle. >> >>> Atheros 11n hardware only supports 1-stream STBC RX. >> >> Did you got my email with lots of assumptions and questions? >> What do you mean by 1-stream STBC RX? After i did some home work on STBC >> i see that it encoded from at least two spatial streams. >> Is >> 1-stream STBC RX = 2 spatial streams with mirrored data? >> and >> 2-stream STBC RX = 4 spatial streams with mirrored data? >> >> or >> >> 1-stream STBC RX = compatibility mode for one stream hardware(so only of >> two streams received)? > When you're talking about 'streams', please specify where you're talking > about Spatial Streams (Nss, defined by the MCS), or Space-Time Streams > (Nsts). STBC is useful whenever the number of possible Space-Time > streams exceeds the number of Spatial streams, i.e. if the number of tx > chains is bigger than the number of spatial streams. > There's an asymmetry between Rx and Tx here. If a receiver has 1 chain > and the transmitter has 2 chains, tx can use 2 Space-Time streams to > encode 1 Spatial stream to improve the reliability of the signal. > The HT STBC capability field indicates the maximum number of Spatial > Streams, not Space-Time streams. Atheros hardware only supports STBC > with Nss = 1, so announcing 2-stream STBC is definitely wrong. Ok, i finally found it on ieee 802.11 specification. For STBC: Nsts=2 - Nss=1 Nsts=3 - Nss=2 Nsts=4 - Nss=2 Nsts=4 - Nss=3 >> That would make sense for 1x1:1 hardware, but if you say all atheros N >> hardware support only 1-stream STBC RX, will mean that STBC is useless >> on this hardware. > Only STBC With Nss=1, Nsts=2 is supported, but this does not make it > useless at all. It helps, even if the receiver only has one antenna. Found it too.. :) Thx! -- Regards, Oleksij ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
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. -- Regards, Oleksij ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
Am 04.05.2013 08:50, schrieb Oleksij Rempel: > 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; > oops... my error. I see the problem now. I do not pass flags provided by peer. -- Regards, Oleksij ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
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. 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. The only STBC related flag that actually gets used (when passed from the driver) is ATH_RC_RX_STBC_FLAG. - Felix ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
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
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
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.
- Felix
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
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
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
On 4 May 2013 04:16, Felix Fietkau wrote: >> 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. It smells like left-over from the 7.x driver code this is based off of. Adrian ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
I've just reviewed it myself: * oan->stbc is enabled only if the hardware itself supports STBC; so it's an extra sanity check in case the firmware is told to enable STBC in the WMI capflag field. * is oan->htinfo used anywhere that may involve STBC? * .. we should check whether ath9k_htc ever set the STBC flags on AR9271, or we'd end up confusing the hardware. * .. I don't think that is important though, as we weren't _doing_ STBC, right? * Why are the ATH_RC_* flags used in newassoc_11n? This comes from the WMI WMI_RC_STATE_CHANGE_CMDID capflag field; where are _those_ defined? Grr, so many things to tidy up. Adrian ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH 1/2] ath9k_htc: add STBC TX support
Am 04.05.2013 19:50, schrieb Adrian Chadd:
I've just reviewed it myself:
* oan->stbc is enabled only if the hardware itself supports STBC; so
it's an extra sanity check in case the firmware is told to enable STBC
in the WMI capflag field.
all STBC parts are not compiled for AR9271. Currently firmware will do
sanity check on htc_7010.fw and always return 1, and wont do any check
on htc_9271.fw (this part is just not compiled).
* is oan->htinfo used anywhere that may involve STBC?
hmm...oan->htinfo?
* .. we should check whether ath9k_htc ever set the STBC flags on
AR9271, or we'd end up confusing the hardware.
no, never. there are legion guardians ;) last on is on preparing tx
descriptor.
* .. I don't think that is important though, as we weren't _doing_ STBC, right?
correct, currently STBC is not working on linux ath9k_htc
* Why are the ATH_RC_* flags used in newassoc_11n? This comes from the
WMI WMI_RC_STATE_CHANGE_CMDID capflag field; where are _those_
defined?
they are in target_firmware/wlan/if_athrate.h
and well, there are a bit more problems thin them.
See attachment :) Attached patch can go on top of my merge request for
firmware. So, i will probably need to make Patch_v5 for ath9k_htc... since.
Felix,
In addition to that, using the WLAN_RC_* flags is wrong, you need to use
the ATH_RC_* flags,...
After some firmware cleaning i will need to use WLAN_RC_* flags :)
WLAN_RC_* have same values like ATH_RC_*...
Grr, so many things to tidy up.
--
Regards,
Oleksij
diff --git a/target_firmware/wlan/ratectrl11n.h b/target_firmware/wlan/ratectrl11n.h
index 64d0197..00fd156 100755
--- a/target_firmware/wlan/ratectrl11n.h
+++ b/target_firmware/wlan/ratectrl11n.h
@@ -156,20 +156,6 @@ typedef struct {
} RATE_TABLE_11N;
/*
- * Update the SIB's rate control information
- *
- * This should be called when the supported rates change
- * (e.g. SME operation, wireless mode change)
- *
- * It will determine which rates are valid for use.
- */
-void rcSibUpdate_11n(struct ath_softc_tgt *,
- struct ath_node_target *,
- A_UINT32 capflag,
- A_BOOL keepState,
- struct ieee80211_rate *rs);
-
-/*
* Determines and returns the new Tx rate index.
*/
void rcRateFind_11n(struct ath_softc_tgt *sc,
diff --git a/target_firmware/wlan/ratectrl_11n_ln.c b/target_firmware/wlan/ratectrl_11n_ln.c
index 52c1fc7..e64b254 100755
--- a/target_firmware/wlan/ratectrl_11n_ln.c
+++ b/target_firmware/wlan/ratectrl_11n_ln.c
@@ -429,19 +429,7 @@ rcSibUpdate_ht(struct ath_softc_tgt *sc, struct ath_node_target *an,
rcSortValidRates(pRateTable, pRc);
}
-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),
- keepState,
- pRateSet);
-}
+
/*
* Return the median of three numbers
@@ -1212,17 +1200,11 @@ ath_rate_newassoc_11n(struct ath_softc_tgt *sc, struct ath_node_target *an, int
if (isnew) {
struct atheros_node *oan = ATH_NODE_ATHEROS(an);
- oan->htcap = ((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_WEP_TKIP_FLAG) ? WLAN_RC_WEP_TKIP_FLAG : 0);
-
#ifdef MAGPIE_MERLIN
/* Only MERLIN can send STBC */
oan->stbc = (capflag & ATH_RC_RX_STBC_FLAG) ? 1 : 0;
#endif
- rcSibUpdate_11n(sc, an, oan->htcap, 0, rs);
+ rcSibUpdate_ht(sc, an, capflag, 0, rs);
}
}
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel
