[PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Felix Fietkau]

Cc: stable@vger.kernel.org
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/ar9003_calib.c | 2 ++
 drivers/net/wireless/ath/ath9k/ar9003_phy.c   | 2 +-
 drivers/net/wireless/ath/ath9k/hw.h           | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_calib.c b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
index 8b0d8dc..56317b0 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
@@ -976,6 +976,8 @@ static bool ar9003_hw_init_cal(struct ath_hw *ah,
 					  AR_PHY_CL_TAB_1,
 					  AR_PHY_CL_TAB_2 };
 
+	ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask, ah->caps.tx_chainmask);
+
 	if (rtt) {
 		if (!ar9003_hw_rtt_restore(ah, chan))
 			run_rtt_cal = true;
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_phy.c b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
index ce19c09..8290edd 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
@@ -586,7 +586,7 @@ static void ar9003_hw_init_bb(struct ath_hw *ah,
 	ath9k_hw_synth_delay(ah, chan, synthDelay);
 }
 
-static void ar9003_hw_set_chain_masks(struct ath_hw *ah, u8 rx, u8 tx)
+void ar9003_hw_set_chain_masks(struct ath_hw *ah, u8 rx, u8 tx)
 {
 	switch (rx) {
 	case 0x5:
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 7f1a8e9..9d26fc5 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -1066,6 +1066,7 @@ void ar9003_paprd_setup_gain_table(struct ath_hw *ah, int chain);
 int ar9003_paprd_init_table(struct ath_hw *ah);
 bool ar9003_paprd_is_done(struct ath_hw *ah);
 bool ar9003_is_paprd_enabled(struct ath_hw *ah);
+void ar9003_hw_set_chain_masks(struct ath_hw *ah, u8 rx, u8 tx);
 
 /* Hardware family op attach helpers */
 void ar5008_hw_attach_phy_ops(struct ath_hw *ah);
-- 
1.8.0.2

[PATCH 3.8 2/3] ath9k_hw: fix chain swap setting when setting rx chainmask to 5

[Felix Fietkau]

Chain swapping should only be enabled when the EEPROM chainmask is set to 5,
regardless of what the runtime chainmask is.

Cc: stable@vger.kernel.org
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/ar9003_phy.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_phy.c b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
index 8290edd..3afc24b 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
@@ -588,30 +588,17 @@ static void ar9003_hw_init_bb(struct ath_hw *ah,
 
 void ar9003_hw_set_chain_masks(struct ath_hw *ah, u8 rx, u8 tx)
 {
-	switch (rx) {
-	case 0x5:
+	if (ah->caps.tx_chainmask == 5 || ah->caps.rx_chainmask == 5)
 		REG_SET_BIT(ah, AR_PHY_ANALOG_SWAP,
 			    AR_PHY_SWAP_ALT_CHAIN);
-	case 0x3:
-	case 0x1:
-	case 0x2:
-	case 0x7:
-		REG_WRITE(ah, AR_PHY_RX_CHAINMASK, rx);
-		REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, rx);
-		break;
-	default:
-		break;
-	}
+
+	REG_WRITE(ah, AR_PHY_RX_CHAINMASK, rx);
+	REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, rx);
 
 	if ((ah->caps.hw_caps & ATH9K_HW_CAP_APM) && (tx == 0x7))
-		REG_WRITE(ah, AR_SELFGEN_MASK, 0x3);
-	else
-		REG_WRITE(ah, AR_SELFGEN_MASK, tx);
+		tx = 3;
 
-	if (tx == 0x5) {
-		REG_SET_BIT(ah, AR_PHY_ANALOG_SWAP,
-			    AR_PHY_SWAP_ALT_CHAIN);
-	}
+	REG_WRITE(ah, AR_SELFGEN_MASK, tx);
 }
 
 /*
-- 
1.8.0.2

[PATCH 3.8 3/3] ath9k: allow setting arbitrary antenna masks on AR9003+

[Felix Fietkau]

Cc: stable@vger.kernel.org
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 3796e65..dd91f8f 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1826,6 +1826,9 @@ static u32 fill_chainmask(u32 cap, u32 new)
 
 static bool validate_antenna_mask(struct ath_hw *ah, u32 val)
 {
+	if (AR_SREV_9300_20_OR_LATER(ah))
+		return true;
+
 	switch (val & 0x7) {
 	case 0x1:
 	case 0x3:
-- 
1.8.0.2

Re: [PATCH 3.8 2/3] ath9k_hw: fix chain swap setting when setting rx chainmask to 5

[Adrian Chadd]

Hiya,

What's the motivation behind this?

The reason why the PHY analog swapping is done when the chainmask set
to 0x5 is because pre-AR9280 chips didn't support arbitrary chainmask
configurations for TX.
So it could either be 0x1, 0x3, or 0x7.

The AR5416 2-TX 3-TX reference design has TX antenna on chain 0 and 2
(hence 0x5, as you know), so:

* One should never be allowed to configure a TX chainmask enabling
chain 1, as that plain won't work (there's no actual TX bits on the
card itself; it only has the RX related switch/amp/etc);
* If the chainmask is set to 0x3 or 0x5 on a device that has 3 TX
chains (ie, anything AR93xx and later with 3 chains and 3 TX capable
switches/amplifiers on the NIC itself) then _strictly speaking_ I
don't think it's necessary to do the analog swap, but that's when
you'd do it.

The AR9130 and AR9160 have the same limitation. The AR9280 and later
don't, but the AR92xx chips are one or two chain, so this doesn't
(strictly speaking) need to happen.

For FreeBSD, I'm going to add code to enforce that the configured
chainmask doesn't conflict with the EEPROM chainmask, in case people
try to do stupid things.

Thanks,

Adrian


On 20 January 2013 12:55, Felix Fietkau <nbd@openwrt.org> wrote:
> Chain swapping should only be enabled when the EEPROM chainmask is set to 5,
> regardless of what the runtime chainmask is.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
>  drivers/net/wireless/ath/ath9k/ar9003_phy.c | 25 ++++++-------------------
>  1 file changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_phy.c b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
> index 8290edd..3afc24b 100644
> --- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
> @@ -588,30 +588,17 @@ static void ar9003_hw_init_bb(struct ath_hw *ah,
>
>  void ar9003_hw_set_chain_masks(struct ath_hw *ah, u8 rx, u8 tx)
>  {
> -       switch (rx) {
> -       case 0x5:
> +       if (ah->caps.tx_chainmask == 5 || ah->caps.rx_chainmask == 5)
>                 REG_SET_BIT(ah, AR_PHY_ANALOG_SWAP,
>                             AR_PHY_SWAP_ALT_CHAIN);
> -       case 0x3:
> -       case 0x1:
> -       case 0x2:
> -       case 0x7:
> -               REG_WRITE(ah, AR_PHY_RX_CHAINMASK, rx);
> -               REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, rx);
> -               break;
> -       default:
> -               break;
> -       }
> +
> +       REG_WRITE(ah, AR_PHY_RX_CHAINMASK, rx);
> +       REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, rx);
>
>         if ((ah->caps.hw_caps & ATH9K_HW_CAP_APM) && (tx == 0x7))
> -               REG_WRITE(ah, AR_SELFGEN_MASK, 0x3);
> -       else
> -               REG_WRITE(ah, AR_SELFGEN_MASK, tx);
> +               tx = 3;
>
> -       if (tx == 0x5) {
> -               REG_SET_BIT(ah, AR_PHY_ANALOG_SWAP,
> -                           AR_PHY_SWAP_ALT_CHAIN);
> -       }
> +       REG_WRITE(ah, AR_SELFGEN_MASK, tx);
>  }
>
>  /*
> --
> 1.8.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.8 2/3] ath9k_hw: fix chain swap setting when setting rx chainmask to 5

[Felix Fietkau]

On 2013-01-20 11:05 PM, Adrian Chadd wrote:
> Hiya,
> 
> What's the motivation behind this?
> 
> The reason why the PHY analog swapping is done when the chainmask set
> to 0x5 is because pre-AR9280 chips didn't support arbitrary chainmask
> configurations for TX.
> So it could either be 0x1, 0x3, or 0x7.
>
> The AR5416 2-TX 3-TX reference design has TX antenna on chain 0 and 2
> (hence 0x5, as you know), so:
> 
> * One should never be allowed to configure a TX chainmask enabling
> chain 1, as that plain won't work (there's no actual TX bits on the
> card itself; it only has the RX related switch/amp/etc);
> * If the chainmask is set to 0x3 or 0x5 on a device that has 3 TX
> chains (ie, anything AR93xx and later with 3 chains and 3 TX capable
> switches/amplifiers on the NIC itself) then _strictly speaking_ I
> don't think it's necessary to do the analog swap, but that's when
> you'd do it.
The analog swapping was already there, I just changed it from being
enabled based on the runtime chainmask to being enabled based on the
EEPROM chainmask. I don't know if the analog swapping is still necessary
or not, but I think it should not be enabled on a 3x3 card when
disabling chain 1.

> The AR9130 and AR9160 have the same limitation. The AR9280 and later
> don't, but the AR92xx chips are one or two chain, so this doesn't
> (strictly speaking) need to happen.
> 
> For FreeBSD, I'm going to add code to enforce that the configured
> chainmask doesn't conflict with the EEPROM chainmask, in case people
> try to do stupid things.
The code in ath9k already does that when it calculates the runtime
chainmask from the user's antenna selection, the user does not set the
chainmask directly.

- Felix

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

There is a regression introduced by this patch when power save is off on
the station for idle checks.
I have AR9590 station with rx and tx chain set to 0x1 connected
to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.

Before this patch in connection polling the station was properly sending
null function to check whether AP is still there. After this patch it sends
broadcast probe request which is anyway wrong or some 16 or so packets
of random data (rarely). It manifests itself in lost connection because 
there
is no ack from AP which is expected for null function.

I have been following skb's up to the descriptor setting in ath9k and it was
all ok i.e. proper null function with valid addresses.

I have been bisecting it twice because it doesn't make much sense but maybe
it's a HW issue?


Wojtek

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Felix Fietkau]

On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
> There is a regression introduced by this patch when power save is off on
> the station for idle checks.
> I have AR9590 station with rx and tx chain set to 0x1 connected
> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
> 
> Before this patch in connection polling the station was properly sending
> null function to check whether AP is still there. After this patch it sends
> broadcast probe request which is anyway wrong or some 16 or so packets
> of random data (rarely). It manifests itself in lost connection because 
> there
> is no ack from AP which is expected for null function.
> 
> I have been following skb's up to the descriptor setting in ath9k and it was
> all ok i.e. proper null function with valid addresses.
> 
> I have been bisecting it twice because it doesn't make much sense but maybe
> it's a HW issue?
You're right, it does not make much sense. I can't figure out how this
patch could possibly change the runtime behavior with tx chainmask set
to 0x1. Have you tried reverting this patch in a current build to see if
that fixes the issue?

- Felix

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/07/2013 03:59 PM, Felix Fietkau wrote:
> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>> There is a regression introduced by this patch when power save is off on
>> the station for idle checks.
>> I have AR9590 station with rx and tx chain set to 0x1 connected
>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>
>> Before this patch in connection polling the station was properly sending
>> null function to check whether AP is still there. After this patch it sends
>> broadcast probe request which is anyway wrong or some 16 or so packets
>> of random data (rarely). It manifests itself in lost connection because
>> there
>> is no ack from AP which is expected for null function.
>>
>> I have been following skb's up to the descriptor setting in ath9k and it was
>> all ok i.e. proper null function with valid addresses.
>>
>> I have been bisecting it twice because it doesn't make much sense but maybe
>> it's a HW issue?
> You're right, it does not make much sense. I can't figure out how this
> patch could possibly change the runtime behavior with tx chainmask set
> to 0x1. Have you tried reverting this patch in a current build to see if
> that fixes the issue?
It does fix it. I will check tomorrow whether it's only AR9590 or also
previous revisions. I will also try with different chainmasks. I will have
to rescrew my setup...

Wojtek

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/07/2013 04:46 PM, Wojciech Dubowik wrote:
> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>> There is a regression introduced by this patch when power save is 
>>> off on
>>> the station for idle checks.
>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>
>>> Before this patch in connection polling the station was properly 
>>> sending
>>> null function to check whether AP is still there. After this patch 
>>> it sends
>>> broadcast probe request which is anyway wrong or some 16 or so packets
>>> of random data (rarely). It manifests itself in lost connection because
>>> there
>>> is no ack from AP which is expected for null function.
>>>
>>> I have been following skb's up to the descriptor setting in ath9k 
>>> and it was
>>> all ok i.e. proper null function with valid addresses.
>>>
>>> I have been bisecting it twice because it doesn't make much sense 
>>> but maybe
>>> it's a HW issue?
>> You're right, it does not make much sense. I can't figure out how this
>> patch could possibly change the runtime behavior with tx chainmask set
>> to 0x1. Have you tried reverting this patch in a current build to see if
>> that fixes the issue?
> It does fix it. I will check tomorrow whether it's only AR9590 or also
> previous revisions. I will also try with different chainmasks. I will 
> have
> to rescrew my setup...
>
I have been doing some tests and it seems to affect both AR9390 and AR9590.
To summarize: sta doesn't send null function but broadcast probe request or
corrupted frames in idle checking routine when power save is off and 
only some
antennas are selected for transmission.
So for example when I set antenna mask 7 i.e. all available it works ok 
but with
mask 1, 3 or 6 not. When I swith power save it's all ok no matter what 
mask I use.

Thing with power save on is that before it goes to idle check it will go 
to sleep since
there is no traffic anyway, then we get beacon miss, it wakes up and it 
sends null
function. I guess waking up is reinitializing sth in a chip which 
doesn't occur in my
scenario.
HW issue?

Wojtek

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/08/2013 08:44 AM, Wojciech Dubowik wrote:
> On 03/07/2013 04:46 PM, Wojciech Dubowik wrote:
>> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>>> There is a regression introduced by this patch when power save is 
>>>> off on
>>>> the station for idle checks.
>>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>>
>>>> Before this patch in connection polling the station was properly 
>>>> sending
>>>> null function to check whether AP is still there. After this patch 
>>>> it sends
>>>> broadcast probe request which is anyway wrong or some 16 or so packets
>>>> of random data (rarely). It manifests itself in lost connection 
>>>> because
>>>> there
>>>> is no ack from AP which is expected for null function.
>>>>
>>>> I have been following skb's up to the descriptor setting in ath9k 
>>>> and it was
>>>> all ok i.e. proper null function with valid addresses.
>>>>
>>>> I have been bisecting it twice because it doesn't make much sense 
>>>> but maybe
>>>> it's a HW issue?
>>> You're right, it does not make much sense. I can't figure out how this
>>> patch could possibly change the runtime behavior with tx chainmask set
>>> to 0x1. Have you tried reverting this patch in a current build to 
>>> see if
>>> that fixes the issue?
>> It does fix it. I will check tomorrow whether it's only AR9590 or also
>> previous revisions. I will also try with different chainmasks. I will 
>> have
>> to rescrew my setup...
>>
> I have been doing some tests and it seems to affect both AR9390 and 
> AR9590.
> To summarize: sta doesn't send null function but broadcast probe 
> request or
> corrupted frames in idle checking routine when power save is off and 
> only some
> antennas are selected for transmission.
> So for example when I set antenna mask 7 i.e. all available it works 
> ok but with
> mask 1, 3 or 6 not. When I swith power save it's all ok no matter what 
> mask I use.
>
> Thing with power save on is that before it goes to idle check it will 
> go to sleep since
> there is no traffic anyway, then we get beacon miss, it wakes up and 
> it sends null
> function. I guess waking up is reinitializing sth in a chip which 
> doesn't occur in my
> scenario.
> HW issue?
It will also work if I set user specified antenna masks instead of hw 
capabilities.
Shouldn't it be the case anyway?

Wojtek

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Felix Fietkau]

On 2013-03-08 10:46 AM, Wojciech Dubowik wrote:
> On 03/08/2013 08:44 AM, Wojciech Dubowik wrote:
>> On 03/07/2013 04:46 PM, Wojciech Dubowik wrote:
>>> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>>>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>>>> There is a regression introduced by this patch when power save is 
>>>>> off on
>>>>> the station for idle checks.
>>>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>>>
>>>>> Before this patch in connection polling the station was properly 
>>>>> sending
>>>>> null function to check whether AP is still there. After this patch 
>>>>> it sends
>>>>> broadcast probe request which is anyway wrong or some 16 or so packets
>>>>> of random data (rarely). It manifests itself in lost connection 
>>>>> because
>>>>> there
>>>>> is no ack from AP which is expected for null function.
>>>>>
>>>>> I have been following skb's up to the descriptor setting in ath9k 
>>>>> and it was
>>>>> all ok i.e. proper null function with valid addresses.
>>>>>
>>>>> I have been bisecting it twice because it doesn't make much sense 
>>>>> but maybe
>>>>> it's a HW issue?
>>>> You're right, it does not make much sense. I can't figure out how this
>>>> patch could possibly change the runtime behavior with tx chainmask set
>>>> to 0x1. Have you tried reverting this patch in a current build to 
>>>> see if
>>>> that fixes the issue?
>>> It does fix it. I will check tomorrow whether it's only AR9590 or also
>>> previous revisions. I will also try with different chainmasks. I will 
>>> have
>>> to rescrew my setup...
>>>
>> I have been doing some tests and it seems to affect both AR9390 and 
>> AR9590.
>> To summarize: sta doesn't send null function but broadcast probe 
>> request or
>> corrupted frames in idle checking routine when power save is off and 
>> only some
>> antennas are selected for transmission.
>> So for example when I set antenna mask 7 i.e. all available it works 
>> ok but with
>> mask 1, 3 or 6 not. When I swith power save it's all ok no matter what 
>> mask I use.
>>
>> Thing with power save on is that before it goes to idle check it will 
>> go to sleep since
>> there is no traffic anyway, then we get beacon miss, it wakes up and 
>> it sends null
>> function. I guess waking up is reinitializing sth in a chip which 
>> doesn't occur in my
>> scenario.
>> HW issue?
> It will also work if I set user specified antenna masks instead of hw 
> capabilities.
What do you mean with that? How do you set the rx and tx chainmasks if
not via antenna masks? debugfs?

- Felix

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/08/2013 01:42 PM, Felix Fietkau wrote:
> On 2013-03-08 10:46 AM, Wojciech Dubowik wrote:
>> On 03/08/2013 08:44 AM, Wojciech Dubowik wrote:
>>> On 03/07/2013 04:46 PM, Wojciech Dubowik wrote:
>>>> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>>>>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>>>>> There is a regression introduced by this patch when power save is
>>>>>> off on
>>>>>> the station for idle checks.
>>>>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>>>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>>>>
>>>>>> Before this patch in connection polling the station was properly
>>>>>> sending
>>>>>> null function to check whether AP is still there. After this patch
>>>>>> it sends
>>>>>> broadcast probe request which is anyway wrong or some 16 or so packets
>>>>>> of random data (rarely). It manifests itself in lost connection
>>>>>> because
>>>>>> there
>>>>>> is no ack from AP which is expected for null function.
>>>>>>
>>>>>> I have been following skb's up to the descriptor setting in ath9k
>>>>>> and it was
>>>>>> all ok i.e. proper null function with valid addresses.
>>>>>>
>>>>>> I have been bisecting it twice because it doesn't make much sense
>>>>>> but maybe
>>>>>> it's a HW issue?
>>>>> You're right, it does not make much sense. I can't figure out how this
>>>>> patch could possibly change the runtime behavior with tx chainmask set
>>>>> to 0x1. Have you tried reverting this patch in a current build to
>>>>> see if
>>>>> that fixes the issue?
>>>> It does fix it. I will check tomorrow whether it's only AR9590 or also
>>>> previous revisions. I will also try with different chainmasks. I will
>>>> have
>>>> to rescrew my setup...
>>>>
>>> I have been doing some tests and it seems to affect both AR9390 and
>>> AR9590.
>>> To summarize: sta doesn't send null function but broadcast probe
>>> request or
>>> corrupted frames in idle checking routine when power save is off and
>>> only some
>>> antennas are selected for transmission.
>>> So for example when I set antenna mask 7 i.e. all available it works
>>> ok but with
>>> mask 1, 3 or 6 not. When I swith power save it's all ok no matter what
>>> mask I use.
>>>
>>> Thing with power save on is that before it goes to idle check it will
>>> go to sleep since
>>> there is no traffic anyway, then we get beacon miss, it wakes up and
>>> it sends null
>>> function. I guess waking up is reinitializing sth in a chip which
>>> doesn't occur in my
>>> scenario.
>>> HW issue?
>> It will also work if I set user specified antenna masks instead of hw
>> capabilities.
> What do you mean with that? How do you set the rx and tx chainmasks if
> not via antenna masks? debugfs?
I do it with iw set antenna.
Wojtek

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/11/2013 07:25 AM, Wojciech Dubowik wrote:
> On 03/08/2013 01:42 PM, Felix Fietkau wrote:
>> On 2013-03-08 10:46 AM, Wojciech Dubowik wrote:
>>> On 03/08/2013 08:44 AM, Wojciech Dubowik wrote:
>>>> On 03/07/2013 04:46 PM, Wojciech Dubowik wrote:
>>>>> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>>>>>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>>>>>> There is a regression introduced by this patch when power save is
>>>>>>> off on
>>>>>>> the station for idle checks.
>>>>>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>>>>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>>>>>
>>>>>>> Before this patch in connection polling the station was properly
>>>>>>> sending
>>>>>>> null function to check whether AP is still there. After this patch
>>>>>>> it sends
>>>>>>> broadcast probe request which is anyway wrong or some 16 or so 
>>>>>>> packets
>>>>>>> of random data (rarely). It manifests itself in lost connection
>>>>>>> because
>>>>>>> there
>>>>>>> is no ack from AP which is expected for null function.
>>>>>>>
>>>>>>> I have been following skb's up to the descriptor setting in ath9k
>>>>>>> and it was
>>>>>>> all ok i.e. proper null function with valid addresses.
>>>>>>>
>>>>>>> I have been bisecting it twice because it doesn't make much sense
>>>>>>> but maybe
>>>>>>> it's a HW issue?
>>>>>> You're right, it does not make much sense. I can't figure out how 
>>>>>> this
>>>>>> patch could possibly change the runtime behavior with tx 
>>>>>> chainmask set
>>>>>> to 0x1. Have you tried reverting this patch in a current build to
>>>>>> see if
>>>>>> that fixes the issue?
>>>>> It does fix it. I will check tomorrow whether it's only AR9590 or 
>>>>> also
>>>>> previous revisions. I will also try with different chainmasks. I will
>>>>> have
>>>>> to rescrew my setup...
>>>>>
>>>> I have been doing some tests and it seems to affect both AR9390 and
>>>> AR9590.
>>>> To summarize: sta doesn't send null function but broadcast probe
>>>> request or
>>>> corrupted frames in idle checking routine when power save is off and
>>>> only some
>>>> antennas are selected for transmission.
>>>> So for example when I set antenna mask 7 i.e. all available it works
>>>> ok but with
>>>> mask 1, 3 or 6 not. When I swith power save it's all ok no matter what
>>>> mask I use.
>>>>
>>>> Thing with power save on is that before it goes to idle check it will
>>>> go to sleep since
>>>> there is no traffic anyway, then we get beacon miss, it wakes up and
>>>> it sends null
>>>> function. I guess waking up is reinitializing sth in a chip which
>>>> doesn't occur in my
>>>> scenario.
>>>> HW issue?
>>> It will also work if I set user specified antenna masks instead of hw
>>> capabilities.
>> What do you mean with that? How do you set the rx and tx chainmasks if
>> not via antenna masks? debugfs?
> I do it with iw set antenna.
This could be the solution if you are ok with it.
Wojtek

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_calib.c 
b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
index 4cc1394..58c6256 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
@@ -1023,7 +1023,7 @@ static bool ar9003_hw_init_cal(struct ath_hw *ah,
AR_PHY_AGC_CONTROL_FLTR_CAL   |
AR_PHY_AGC_CONTROL_PKDET_CAL;

-       ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask, 
ah->caps.tx_chainmask);
+       ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);

         if (rtt) {
                 if (!ar9003_hw_rtt_restore(ah, chan))

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Felix Fietkau]

On 2013-03-11 10:43 AM, Wojciech Dubowik wrote:
> On 03/11/2013 07:25 AM, Wojciech Dubowik wrote:
>> On 03/08/2013 01:42 PM, Felix Fietkau wrote:
>>> On 2013-03-08 10:46 AM, Wojciech Dubowik wrote:
>>>> On 03/08/2013 08:44 AM, Wojciech Dubowik wrote:
>>>>> On 03/07/2013 04:46 PM, Wojciech Dubowik wrote:
>>>>>> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>>>>>>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>>>>>>> There is a regression introduced by this patch when power save is
>>>>>>>> off on
>>>>>>>> the station for idle checks.
>>>>>>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>>>>>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>>>>>>
>>>>>>>> Before this patch in connection polling the station was properly
>>>>>>>> sending
>>>>>>>> null function to check whether AP is still there. After this patch
>>>>>>>> it sends
>>>>>>>> broadcast probe request which is anyway wrong or some 16 or so 
>>>>>>>> packets
>>>>>>>> of random data (rarely). It manifests itself in lost connection
>>>>>>>> because
>>>>>>>> there
>>>>>>>> is no ack from AP which is expected for null function.
>>>>>>>>
>>>>>>>> I have been following skb's up to the descriptor setting in ath9k
>>>>>>>> and it was
>>>>>>>> all ok i.e. proper null function with valid addresses.
>>>>>>>>
>>>>>>>> I have been bisecting it twice because it doesn't make much sense
>>>>>>>> but maybe
>>>>>>>> it's a HW issue?
>>>>>>> You're right, it does not make much sense. I can't figure out how 
>>>>>>> this
>>>>>>> patch could possibly change the runtime behavior with tx 
>>>>>>> chainmask set
>>>>>>> to 0x1. Have you tried reverting this patch in a current build to
>>>>>>> see if
>>>>>>> that fixes the issue?
>>>>>> It does fix it. I will check tomorrow whether it's only AR9590 or 
>>>>>> also
>>>>>> previous revisions. I will also try with different chainmasks. I will
>>>>>> have
>>>>>> to rescrew my setup...
>>>>>>
>>>>> I have been doing some tests and it seems to affect both AR9390 and
>>>>> AR9590.
>>>>> To summarize: sta doesn't send null function but broadcast probe
>>>>> request or
>>>>> corrupted frames in idle checking routine when power save is off and
>>>>> only some
>>>>> antennas are selected for transmission.
>>>>> So for example when I set antenna mask 7 i.e. all available it works
>>>>> ok but with
>>>>> mask 1, 3 or 6 not. When I swith power save it's all ok no matter what
>>>>> mask I use.
>>>>>
>>>>> Thing with power save on is that before it goes to idle check it will
>>>>> go to sleep since
>>>>> there is no traffic anyway, then we get beacon miss, it wakes up and
>>>>> it sends null
>>>>> function. I guess waking up is reinitializing sth in a chip which
>>>>> doesn't occur in my
>>>>> scenario.
>>>>> HW issue?
>>>> It will also work if I set user specified antenna masks instead of hw
>>>> capabilities.
>>> What do you mean with that? How do you set the rx and tx chainmasks if
>>> not via antenna masks? debugfs?
>> I do it with iw set antenna.
> This could be the solution if you are ok with it.
> Wojtek
> 
> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_calib.c 
> b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> index 4cc1394..58c6256 100644
> --- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> @@ -1023,7 +1023,7 @@ static bool ar9003_hw_init_cal(struct ath_hw *ah,
> AR_PHY_AGC_CONTROL_FLTR_CAL   |
> AR_PHY_AGC_CONTROL_PKDET_CAL;
> 
> -       ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask, 
> ah->caps.tx_chainmask);
> +       ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);
I will do some testing, but I think this will probably be equivalent to
a revert of the patch.

- Felix

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Felix Fietkau]

On 2013-03-15 1:01 AM, Felix Fietkau wrote:
> On 2013-03-11 10:43 AM, Wojciech Dubowik wrote:
>> This could be the solution if you are ok with it.
>> Wojtek
>> 
>> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_calib.c 
>> b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>> index 4cc1394..58c6256 100644
>> --- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>> +++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>> @@ -1023,7 +1023,7 @@ static bool ar9003_hw_init_cal(struct ath_hw *ah,
>> AR_PHY_AGC_CONTROL_FLTR_CAL   |
>> AR_PHY_AGC_CONTROL_PKDET_CAL;
>> 
>> -       ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask, 
>> ah->caps.tx_chainmask);
>> +       ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);
> I will do some testing, but I think this will probably be equivalent to
> a revert of the patch.
Please try this patch instead:
---
--- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
@@ -1023,6 +1023,7 @@ static bool ar9003_hw_init_cal(struct at
 					  AR_PHY_AGC_CONTROL_FLTR_CAL   |
 					  AR_PHY_AGC_CONTROL_PKDET_CAL;
 
+	/* Use chip chainmask only for calibration */
 	ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask, ah->caps.tx_chainmask);
 
 	if (rtt) {
@@ -1150,6 +1151,9 @@ skip_tx_iqcal:
 		ar9003_hw_rtt_disable(ah);
 	}
 
+	/* Revert chainmask to runtime parameters */
+	ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);
+
 	/* Initialize list pointers */
 	ah->cal_list = ah->cal_list_last = ah->cal_list_curr = NULL;
 

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/15/2013 02:32 AM, Felix Fietkau wrote:
> On 2013-03-15 1:01 AM, Felix Fietkau wrote:
>> On 2013-03-11 10:43 AM, Wojciech Dubowik wrote:
>>> This could be the solution if you are ok with it.
>>> Wojtek
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>>> b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>>> index 4cc1394..58c6256 100644
>>> --- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>>> +++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
>>> @@ -1023,7 +1023,7 @@ static bool ar9003_hw_init_cal(struct ath_hw *ah,
>>> AR_PHY_AGC_CONTROL_FLTR_CAL   |
>>> AR_PHY_AGC_CONTROL_PKDET_CAL;
>>>
>>> -       ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask,
>>> ah->caps.tx_chainmask);
>>> +       ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);
>> I will do some testing, but I think this will probably be equivalent to
>> a revert of the patch.
> Please try this patch instead:
> ---
> --- a/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_calib.c
> @@ -1023,6 +1023,7 @@ static bool ar9003_hw_init_cal(struct at
>   					  AR_PHY_AGC_CONTROL_FLTR_CAL   |
>   					  AR_PHY_AGC_CONTROL_PKDET_CAL;
>   
> +	/* Use chip chainmask only for calibration */
>   	ar9003_hw_set_chain_masks(ah, ah->caps.rx_chainmask, ah->caps.tx_chainmask);
>   
>   	if (rtt) {
> @@ -1150,6 +1151,9 @@ skip_tx_iqcal:
>   		ar9003_hw_rtt_disable(ah);
>   	}
>   
> +	/* Revert chainmask to runtime parameters */
> +	ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);
> +
>   	/* Initialize list pointers */
>   	ah->cal_list = ah->cal_list_last = ah->cal_list_curr = NULL;
>   
It works. Thanks for looking at it.

Wojtek

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[John W. Linville]

On Thu, Mar 07, 2013 at 04:46:30PM +0100, Wojciech Dubowik wrote:
> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
> >On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
> >>There is a regression introduced by this patch when power save is off on
> >>the station for idle checks.
> >>I have AR9590 station with rx and tx chain set to 0x1 connected
> >>to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
> >>
> >>Before this patch in connection polling the station was properly sending
> >>null function to check whether AP is still there. After this patch it sends
> >>broadcast probe request which is anyway wrong or some 16 or so packets
> >>of random data (rarely). It manifests itself in lost connection because
> >>there
> >>is no ack from AP which is expected for null function.
> >>
> >>I have been following skb's up to the descriptor setting in ath9k and it was
> >>all ok i.e. proper null function with valid addresses.
> >>
> >>I have been bisecting it twice because it doesn't make much sense but maybe
> >>it's a HW issue?
> >You're right, it does not make much sense. I can't figure out how this
> >patch could possibly change the runtime behavior with tx chainmask set
> >to 0x1. Have you tried reverting this patch in a current build to see if
> >that fixes the issue?
> It does fix it. I will check tomorrow whether it's only AR9590 or also
> previous revisions. I will also try with different chainmasks. I will have
> to rescrew my setup...

Do I need to revert this one?  Or is there a new fix coming?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH 3.8 1/3] ath9k_hw: fix calibration issues on chainmask that don't include chain 0

[Wojciech Dubowik]

On 03/08/2013 09:34 PM, John W. Linville wrote:
> On Thu, Mar 07, 2013 at 04:46:30PM +0100, Wojciech Dubowik wrote:
>> On 03/07/2013 03:59 PM, Felix Fietkau wrote:
>>> On 2013-03-07 3:31 PM, Wojciech Dubowik wrote:
>>>> There is a regression introduced by this patch when power save is off on
>>>> the station for idle checks.
>>>> I have AR9590 station with rx and tx chain set to 0x1 connected
>>>> to legacy AP (based on Ar9390) with RF cable and 40dB attenuator.
>>>>
>>>> Before this patch in connection polling the station was properly sending
>>>> null function to check whether AP is still there. After this patch it sends
>>>> broadcast probe request which is anyway wrong or some 16 or so packets
>>>> of random data (rarely). It manifests itself in lost connection because
>>>> there
>>>> is no ack from AP which is expected for null function.
>>>>
>>>> I have been following skb's up to the descriptor setting in ath9k and it was
>>>> all ok i.e. proper null function with valid addresses.
>>>>
>>>> I have been bisecting it twice because it doesn't make much sense but maybe
>>>> it's a HW issue?
>>> You're right, it does not make much sense. I can't figure out how this
>>> patch could possibly change the runtime behavior with tx chainmask set
>>> to 0x1. Have you tried reverting this patch in a current build to see if
>>> that fixes the issue?
>> It does fix it. I will check tomorrow whether it's only AR9590 or also
>> previous revisions. I will also try with different chainmasks. I will have
>> to rescrew my setup...
> Do I need to revert this one?  Or is there a new fix coming?
>
I guess we have to figure out first what's going on. I guess in most
cases users select anyway power save, all antennas and don't care
what happens in idle time so they won't see it.

It could be some inconsistency with antenna setting between ath9k
and mac80211. I am looking into it and I guess Felix as well.

Wojtek

Wojtek