* [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan
@ 2010-09-14 15:00 Luis R. Rodriguez
2010-09-14 16:39 ` Luis R. Rodriguez
0 siblings, 1 reply; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-09-14 15:00 UTC (permalink / raw)
To: linville
Cc: linux-wireless, Luis R. Rodriguez, stable, Paul Stewart, Amod Bodas
When we return to the home channel we were never reseting our beacon
timers, this was casued by the fact that the scanning flag was still
on even after we returned to our home channel. There are also other
reasons why we would get a reset and if we are not off channel
we always need to resynch our beacon timers, because a reset will
clear them.
For more details refer to:
http://code.google.com/p/chromium-os/issues/detail?id=5715
This bug is a regression introduced on 2.6.36. The order of the
changes are as follows:
5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
--contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
v2.6.36-rc1~43^2~34^2~22
mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
--contains a0daa0e7592ada797d6835f11529097aabc27ad2
v2.6.36-rc1~571^2~64^2~13
mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
--contains 543708be320d7df692d24b349ca01a947b340764
v2.6.36-rc1~571^2~107^2~187
So 5ee08656 would have worked if a0daa0e7 was not committed but
it was so this means 5ee08656 was broken since it assumed that
when we were in the channel change routine the scan flag would
be lifted. As it turns out the scan flag will be set when we
are already on the home channel.
These issues will need to be considered for our solution on
reshifting the scan complete callback location on mac80211 on
current development kernel work.
This patch has stable fixes which apply down to [2.6.36+]
Cc: stable@kernel.org
Cc: Paul Stewart <pstew@google.com>
Cc: Amod Bodas <amod.bodas@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
This v2 clarifies the second to last sentence, on the v1 this
was just jiberish on my first read.. this should clarify what
I meant.
drivers/net/wireless/ath/ath9k/main.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index ba029b2..e349619 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
ath_start_ani(common);
ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
- ath_beacon_config(sc, NULL);
}
+ if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
+ ath_beacon_config(sc, NULL);
+
ps_restore:
ath9k_ps_restore(sc);
return r;
@@ -957,7 +959,7 @@ int ath_reset(struct ath_softc *sc, bool retry_tx)
ath_update_txpow(sc);
- if (sc->sc_flags & SC_OP_BEACONS)
+ if ((sc->sc_flags & SC_OP_BEACONS) || !(sc->sc_flags & (SC_OP_OFFCHANNEL)))
ath_beacon_config(sc, NULL); /* restart beacons */
ath9k_hw_set_interrupts(ah, ah->imask);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan
2010-09-14 15:00 [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan Luis R. Rodriguez
@ 2010-09-14 16:39 ` Luis R. Rodriguez
2010-09-14 16:42 ` Luis R. Rodriguez
0 siblings, 1 reply; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-09-14 16:39 UTC (permalink / raw)
To: linville
Cc: linux-wireless, Luis R. Rodriguez, stable, Paul Stewart, Amod Bodas
On Tue, Sep 14, 2010 at 8:00 AM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> When we return to the home channel we were never reseting our beacon
> timers, this was casued by the fact that the scanning flag was still
> on even after we returned to our home channel. There are also other
> reasons why we would get a reset and if we are not off channel
> we always need to resynch our beacon timers, because a reset will
> clear them.
>
> For more details refer to:
>
> http://code.google.com/p/chromium-os/issues/detail?id=5715
>
> This bug is a regression introduced on 2.6.36. The order of the
> changes are as follows:
>
> 5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
> a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
> 543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
>
> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
> --contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
> v2.6.36-rc1~43^2~34^2~22
>
> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
> --contains a0daa0e7592ada797d6835f11529097aabc27ad2
> v2.6.36-rc1~571^2~64^2~13
>
> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
> --contains 543708be320d7df692d24b349ca01a947b340764
> v2.6.36-rc1~571^2~107^2~187
>
> So 5ee08656 would have worked if a0daa0e7 was not committed but
> it was so this means 5ee08656 was broken since it assumed that
> when we were in the channel change routine the scan flag would
> be lifted. As it turns out the scan flag will be set when we
> are already on the home channel.
>
> These issues will need to be considered for our solution on
> reshifting the scan complete callback location on mac80211 on
> current development kernel work.
>
> This patch has stable fixes which apply down to [2.6.36+]
>
> Cc: stable@kernel.org
> Cc: Paul Stewart <pstew@google.com>
> Cc: Amod Bodas <amod.bodas@atheros.com>
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
>
> This v2 clarifies the second to last sentence, on the v1 this
> was just jiberish on my first read.. this should clarify what
> I meant.
>
> drivers/net/wireless/ath/ath9k/main.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index ba029b2..e349619 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
> if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
> ath_start_ani(common);
> ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
> - ath_beacon_config(sc, NULL);
> }
>
> + if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
> + ath_beacon_config(sc, NULL);
> +
Upon further thought this change should just remove the SC_OP_SCANNING
flag from above and let the code there run when we are not off
channel. Reason is that as I noted above on the commit log entry the
scan flag will be set even when we return to the home channel due to
the races in mac80211 that were actually fixed by Johannes but later
reverted due to issues. Doing the fix this way would also let us
re-arm ANI and the tx completion monitor, this was also broken! I'll
respin this patch shortly after some basic testing.
Luis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan
2010-09-14 16:39 ` Luis R. Rodriguez
@ 2010-09-14 16:42 ` Luis R. Rodriguez
2010-09-14 17:56 ` Luis R. Rodriguez
0 siblings, 1 reply; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-09-14 16:42 UTC (permalink / raw)
To: linville
Cc: linux-wireless, Luis R. Rodriguez, stable, Paul Stewart, Amod Bodas
On Tue, Sep 14, 2010 at 9:39 AM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> On Tue, Sep 14, 2010 at 8:00 AM, Luis R. Rodriguez
> <lrodriguez@atheros.com> wrote:
>> When we return to the home channel we were never reseting our beacon
>> timers, this was casued by the fact that the scanning flag was still
>> on even after we returned to our home channel. There are also other
>> reasons why we would get a reset and if we are not off channel
>> we always need to resynch our beacon timers, because a reset will
>> clear them.
>>
>> For more details refer to:
>>
>> http://code.google.com/p/chromium-os/issues/detail?id=5715
>>
>> This bug is a regression introduced on 2.6.36. The order of the
>> changes are as follows:
>>
>> 5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
>> a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
>> 543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
>>
>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> --contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
>> v2.6.36-rc1~43^2~34^2~22
>>
>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> --contains a0daa0e7592ada797d6835f11529097aabc27ad2
>> v2.6.36-rc1~571^2~64^2~13
>>
>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> --contains 543708be320d7df692d24b349ca01a947b340764
>> v2.6.36-rc1~571^2~107^2~187
>>
>> So 5ee08656 would have worked if a0daa0e7 was not committed but
>> it was so this means 5ee08656 was broken since it assumed that
>> when we were in the channel change routine the scan flag would
>> be lifted. As it turns out the scan flag will be set when we
>> are already on the home channel.
>>
>> These issues will need to be considered for our solution on
>> reshifting the scan complete callback location on mac80211 on
>> current development kernel work.
>>
>> This patch has stable fixes which apply down to [2.6.36+]
>>
>> Cc: stable@kernel.org
>> Cc: Paul Stewart <pstew@google.com>
>> Cc: Amod Bodas <amod.bodas@atheros.com>
>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>> ---
>>
>> This v2 clarifies the second to last sentence, on the v1 this
>> was just jiberish on my first read.. this should clarify what
>> I meant.
>>
>> drivers/net/wireless/ath/ath9k/main.c | 6 ++++--
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index ba029b2..e349619 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>> if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
>> ath_start_ani(common);
>> ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
>> - ath_beacon_config(sc, NULL);
>> }
>>
>> + if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
>> + ath_beacon_config(sc, NULL);
>> +
>
> Upon further thought this change should just remove the SC_OP_SCANNING
> flag from above and let the code there run when we are not off
> channel. Reason is that as I noted above on the commit log entry the
> scan flag will be set even when we return to the home channel due to
> the races in mac80211 that were actually fixed by Johannes but later
> reverted due to issues. Doing the fix this way would also let us
> re-arm ANI and the tx completion monitor, this was also broken! I'll
> respin this patch shortly after some basic testing.
I should also note that this patch fixes the issue of reseting the
beacon timers upon a reset, likely when the tx monitor hits -- I
suspect that when that currently would hit we'd actually be out of
synch with the AP and disconnect. This patch fixes that, but I just
reviewed older kernels and the offchannel flag was not available prior
to 2.6.36 so we can only fix this properly on >= 2.6.36 unless we
figure out a way to annotate we're on the home channel. If we do we
can potentially propagate a fix down to older kernels than 2.6.36
eventually.
Luis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan
2010-09-14 16:42 ` Luis R. Rodriguez
@ 2010-09-14 17:56 ` Luis R. Rodriguez
2010-09-14 20:06 ` Luis R. Rodriguez
0 siblings, 1 reply; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-09-14 17:56 UTC (permalink / raw)
To: linville
Cc: linux-wireless, Luis R. Rodriguez, stable, Paul Stewart, Amod Bodas
On Tue, Sep 14, 2010 at 9:42 AM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> On Tue, Sep 14, 2010 at 9:39 AM, Luis R. Rodriguez
> <lrodriguez@atheros.com> wrote:
>> On Tue, Sep 14, 2010 at 8:00 AM, Luis R. Rodriguez
>> <lrodriguez@atheros.com> wrote:
>>> When we return to the home channel we were never reseting our beacon
>>> timers, this was casued by the fact that the scanning flag was still
>>> on even after we returned to our home channel. There are also other
>>> reasons why we would get a reset and if we are not off channel
>>> we always need to resynch our beacon timers, because a reset will
>>> clear them.
>>>
>>> For more details refer to:
>>>
>>> http://code.google.com/p/chromium-os/issues/detail?id=5715
>>>
>>> This bug is a regression introduced on 2.6.36. The order of the
>>> changes are as follows:
>>>
>>> 5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
>>> a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
>>> 543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
>>>
>>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>>> --contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
>>> v2.6.36-rc1~43^2~34^2~22
>>>
>>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>>> --contains a0daa0e7592ada797d6835f11529097aabc27ad2
>>> v2.6.36-rc1~571^2~64^2~13
>>>
>>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>>> --contains 543708be320d7df692d24b349ca01a947b340764
>>> v2.6.36-rc1~571^2~107^2~187
>>>
>>> So 5ee08656 would have worked if a0daa0e7 was not committed but
>>> it was so this means 5ee08656 was broken since it assumed that
>>> when we were in the channel change routine the scan flag would
>>> be lifted. As it turns out the scan flag will be set when we
>>> are already on the home channel.
>>>
>>> These issues will need to be considered for our solution on
>>> reshifting the scan complete callback location on mac80211 on
>>> current development kernel work.
>>>
>>> This patch has stable fixes which apply down to [2.6.36+]
>>>
>>> Cc: stable@kernel.org
>>> Cc: Paul Stewart <pstew@google.com>
>>> Cc: Amod Bodas <amod.bodas@atheros.com>
>>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>>> ---
>>>
>>> This v2 clarifies the second to last sentence, on the v1 this
>>> was just jiberish on my first read.. this should clarify what
>>> I meant.
>>>
>>> drivers/net/wireless/ath/ath9k/main.c | 6 ++++--
>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index ba029b2..e349619 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>>> if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
>>> ath_start_ani(common);
>>> ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
>>> - ath_beacon_config(sc, NULL);
>>> }
>>>
>>> + if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
>>> + ath_beacon_config(sc, NULL);
>>> +
>>
>> Upon further thought this change should just remove the SC_OP_SCANNING
>> flag from above and let the code there run when we are not off
>> channel. Reason is that as I noted above on the commit log entry the
>> scan flag will be set even when we return to the home channel due to
>> the races in mac80211 that were actually fixed by Johannes but later
>> reverted due to issues. Doing the fix this way would also let us
>> re-arm ANI and the tx completion monitor, this was also broken! I'll
>> respin this patch shortly after some basic testing.
>
> I should also note that this patch fixes the issue of reseting the
> beacon timers upon a reset, likely when the tx monitor hits -- I
> suspect that when that currently would hit we'd actually be out of
> synch with the AP and disconnect. This patch fixes that, but I just
> reviewed older kernels and the offchannel flag was not available prior
> to 2.6.36 so we can only fix this properly on >= 2.6.36 unless we
> figure out a way to annotate we're on the home channel. If we do we
> can potentially propagate a fix down to older kernels than 2.6.36
> eventually.
Fun, just noticed these would be busted too :)
ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
Ah, joy.
/me fixes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan
2010-09-14 17:56 ` Luis R. Rodriguez
@ 2010-09-14 20:06 ` Luis R. Rodriguez
2010-09-14 22:00 ` Luis R. Rodriguez
0 siblings, 1 reply; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-09-14 20:06 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis Rodriguez, stable, Paul Stewart, Amod Bodas
On Tue, Sep 14, 2010 at 10:56:14AM -0700, Luis R. Rodriguez wrote:
> On Tue, Sep 14, 2010 at 9:42 AM, Luis R. Rodriguez
> <lrodriguez@atheros.com> wrote:
> > On Tue, Sep 14, 2010 at 9:39 AM, Luis R. Rodriguez
> > <lrodriguez@atheros.com> wrote:
> >> On Tue, Sep 14, 2010 at 8:00 AM, Luis R. Rodriguez
> >> <lrodriguez@atheros.com> wrote:
> >>> When we return to the home channel we were never reseting our beacon
> >>> timers, this was casued by the fact that the scanning flag was still
> >>> on even after we returned to our home channel. There are also other
> >>> reasons why we would get a reset and if we are not off channel
> >>> we always need to resynch our beacon timers, because a reset will
> >>> clear them.
> >>>
> >>> For more details refer to:
> >>>
> >>> http://code.google.com/p/chromium-os/issues/detail?id=5715
> >>>
> >>> This bug is a regression introduced on 2.6.36. The order of the
> >>> changes are as follows:
> >>>
> >>> 5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
> >>> a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
> >>> 543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
> >>>
> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
> >>> --contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
> >>> v2.6.36-rc1~43^2~34^2~22
> >>>
> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
> >>> --contains a0daa0e7592ada797d6835f11529097aabc27ad2
> >>> v2.6.36-rc1~571^2~64^2~13
> >>>
> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
> >>> --contains 543708be320d7df692d24b349ca01a947b340764
> >>> v2.6.36-rc1~571^2~107^2~187
> >>>
> >>> So 5ee08656 would have worked if a0daa0e7 was not committed but
> >>> it was so this means 5ee08656 was broken since it assumed that
> >>> when we were in the channel change routine the scan flag would
> >>> be lifted. As it turns out the scan flag will be set when we
> >>> are already on the home channel.
> >>>
> >>> These issues will need to be considered for our solution on
> >>> reshifting the scan complete callback location on mac80211 on
> >>> current development kernel work.
> >>>
> >>> This patch has stable fixes which apply down to [2.6.36+]
> >>>
> >>> Cc: stable@kernel.org
> >>> Cc: Paul Stewart <pstew@google.com>
> >>> Cc: Amod Bodas <amod.bodas@atheros.com>
> >>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> >>> ---
> >>>
> >>> This v2 clarifies the second to last sentence, on the v1 this
> >>> was just jiberish on my first read.. this should clarify what
> >>> I meant.
> >>>
> >>> drivers/net/wireless/ath/ath9k/main.c | 6 ++++--
> >>> 1 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> >>> index ba029b2..e349619 100644
> >>> --- a/drivers/net/wireless/ath/ath9k/main.c
> >>> +++ b/drivers/net/wireless/ath/ath9k/main.c
> >>> @@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
> >>> if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
> >>> ath_start_ani(common);
> >>> ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
> >>> - ath_beacon_config(sc, NULL);
> >>> }
> >>>
> >>> + if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
> >>> + ath_beacon_config(sc, NULL);
> >>> +
> >>
> >> Upon further thought this change should just remove the SC_OP_SCANNING
> >> flag from above and let the code there run when we are not off
> >> channel. Reason is that as I noted above on the commit log entry the
> >> scan flag will be set even when we return to the home channel due to
> >> the races in mac80211 that were actually fixed by Johannes but later
> >> reverted due to issues. Doing the fix this way would also let us
> >> re-arm ANI and the tx completion monitor, this was also broken! I'll
> >> respin this patch shortly after some basic testing.
> >
> > I should also note that this patch fixes the issue of reseting the
> > beacon timers upon a reset, likely when the tx monitor hits -- I
> > suspect that when that currently would hit we'd actually be out of
> > synch with the AP and disconnect. This patch fixes that, but I just
> > reviewed older kernels and the offchannel flag was not available prior
> > to 2.6.36 so we can only fix this properly on >= 2.6.36 unless we
> > figure out a way to annotate we're on the home channel. If we do we
> > can potentially propagate a fix down to older kernels than 2.6.36
> > eventually.
>
> Fun, just noticed these would be busted too :)
>
> ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
>
> Ah, joy.
More joy, so I tried this as an alternative:
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 5fe570b..ecd6724 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -563,7 +563,6 @@ struct ath_ant_comb {
#define SC_OP_RXFLUSH BIT(7)
#define SC_OP_LED_ASSOCIATED BIT(8)
#define SC_OP_LED_ON BIT(9)
-#define SC_OP_SCANNING BIT(10)
#define SC_OP_TSF_RESET BIT(11)
#define SC_OP_BT_PRIORITY_DETECTED BIT(12)
#define SC_OP_BT_SCAN BIT(13)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index ba029b2..2207cf2 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -255,7 +255,7 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
ath_update_txpow(sc);
ath9k_hw_set_interrupts(ah, ah->imask);
- if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
+ if (!(sc->sc_flags & SC_OP_OFFCHANNEL)) {
ath_start_ani(common);
ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
ath_beacon_config(sc, NULL);
@@ -957,7 +957,7 @@ int ath_reset(struct ath_softc *sc, bool retry_tx)
ath_update_txpow(sc);
- if (sc->sc_flags & SC_OP_BEACONS)
+ if ((sc->sc_flags & SC_OP_BEACONS) || !(sc->sc_flags & (SC_OP_OFFCHANNEL)))
ath_beacon_config(sc, NULL); /* restart beacons */
ath9k_hw_set_interrupts(ah, ah->imask);
@@ -2042,7 +2042,6 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
aphy->state = ATH_WIPHY_SCAN;
ath9k_wiphy_pause_all_forced(sc, aphy);
- sc->sc_flags |= SC_OP_SCANNING;
mutex_unlock(&sc->mutex);
}
@@ -2057,7 +2056,6 @@ static void ath9k_sw_scan_complete(struct ieee80211_hw *hw)
mutex_lock(&sc->mutex);
aphy->state = ATH_WIPHY_ACTIVE;
- sc->sc_flags &= ~SC_OP_SCANNING;
mutex_unlock(&sc->mutex);
}
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 87e02f5..6f7d4d1 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -301,7 +301,7 @@ static void ath_edma_start_recv(struct ath_softc *sc)
ath_opmode_init(sc);
- ath9k_hw_startpcureceive(sc->sc_ah, (sc->sc_flags & SC_OP_SCANNING));
+ ath9k_hw_startpcureceive(sc->sc_ah, !!(sc->sc_flags & SC_OP_OFFCHANNEL));
}
static void ath_edma_stop_recv(struct ath_softc *sc)
@@ -507,7 +507,7 @@ int ath_startrecv(struct ath_softc *sc)
start_recv:
spin_unlock_bh(&sc->rx.rxbuflock);
ath_opmode_init(sc);
- ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
+ ath9k_hw_startpcureceive(ah, !!(sc->sc_flags & SC_OP_OFFCHANNEL));
return 0;
}
But the harware goes nutty. Although technically it seems correct I believe the
issue is upon returning to the home channel we actually end up setting the
home channel twice!
Sep 14 13:00:03 tux kernel: [ 309.560059] ath: Set channel: 5785 MHz
Sep 14 13:00:03 tux kernel: [ 309.560067] ath: tx chmask: 3, rx chmask: 3
Sep 14 13:00:03 tux kernel: [ 309.560191] ath: (5765 MHz) -> (5785 MHz), conf_is_ht40: 0 fastcc: 1
Sep 14 13:00:03 tux kernel: [ 309.620100] ath: Set channel: 5785 MHz
Sep 14 13:00:03 tux kernel: [ 309.620110] ath: tx chmask: 3, rx chmask: 3
Sep 14 13:00:03 tux kernel: [ 309.620235] ath: (5785 MHz) -> (5785 MHz), conf_is_ht40: 1 fastcc: 0
Wow, double rainbow all the way [1]! I'll try to see if we can fix this
through a stable kernel, if not the patch I posted earlier is correct and
we'll need more work on this for >= 2.6.37.
[1] http://www.youtube.com/watch?v=OQSNhk5ICTI
Luis
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan
2010-09-14 20:06 ` Luis R. Rodriguez
@ 2010-09-14 22:00 ` Luis R. Rodriguez
0 siblings, 0 replies; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-09-14 22:00 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis Rodriguez, stable, Paul Stewart, Amod Bodas
On Tue, Sep 14, 2010 at 1:06 PM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> On Tue, Sep 14, 2010 at 10:56:14AM -0700, Luis R. Rodriguez wrote:
>> On Tue, Sep 14, 2010 at 9:42 AM, Luis R. Rodriguez
>> <lrodriguez@atheros.com> wrote:
>> > On Tue, Sep 14, 2010 at 9:39 AM, Luis R. Rodriguez
>> > <lrodriguez@atheros.com> wrote:
>> >> On Tue, Sep 14, 2010 at 8:00 AM, Luis R. Rodriguez
>> >> <lrodriguez@atheros.com> wrote:
>> >>> When we return to the home channel we were never reseting our beacon
>> >>> timers, this was casued by the fact that the scanning flag was still
>> >>> on even after we returned to our home channel. There are also other
>> >>> reasons why we would get a reset and if we are not off channel
>> >>> we always need to resynch our beacon timers, because a reset will
>> >>> clear them.
>> >>>
>> >>> For more details refer to:
>> >>>
>> >>> http://code.google.com/p/chromium-os/issues/detail?id=5715
>> >>>
>> >>> This bug is a regression introduced on 2.6.36. The order of the
>> >>> changes are as follows:
>> >>>
>> >>> 5ee08656 - Sat Jul 31 - ath9k: prevent calibration during off-channel activity
>> >>> a0daa0e7 - Tue Jul 27 - Revert "mac80211: fix sw scan bracketing"
>> >>> 543708be - Fri Jun 18 - mac80211: fix sw scan bracketing
>> >>>
>> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> >>> --contains 5ee0865615f65f84e6ee9174771a6716c29e08e1
>> >>> v2.6.36-rc1~43^2~34^2~22
>> >>>
>> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> >>> --contains a0daa0e7592ada797d6835f11529097aabc27ad2
>> >>> v2.6.36-rc1~571^2~64^2~13
>> >>>
>> >>> mcgrof@tux ~/linux-2.6-allstable (git::master)$ git describe \
>> >>> --contains 543708be320d7df692d24b349ca01a947b340764
>> >>> v2.6.36-rc1~571^2~107^2~187
>> >>>
>> >>> So 5ee08656 would have worked if a0daa0e7 was not committed but
>> >>> it was so this means 5ee08656 was broken since it assumed that
>> >>> when we were in the channel change routine the scan flag would
>> >>> be lifted. As it turns out the scan flag will be set when we
>> >>> are already on the home channel.
>> >>>
>> >>> These issues will need to be considered for our solution on
>> >>> reshifting the scan complete callback location on mac80211 on
>> >>> current development kernel work.
>> >>>
>> >>> This patch has stable fixes which apply down to [2.6.36+]
>> >>>
>> >>> Cc: stable@kernel.org
>> >>> Cc: Paul Stewart <pstew@google.com>
>> >>> Cc: Amod Bodas <amod.bodas@atheros.com>
>> >>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>> >>> ---
>> >>>
>> >>> This v2 clarifies the second to last sentence, on the v1 this
>> >>> was just jiberish on my first read.. this should clarify what
>> >>> I meant.
>> >>>
>> >>> drivers/net/wireless/ath/ath9k/main.c | 6 ++++--
>> >>> 1 files changed, 4 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> >>> index ba029b2..e349619 100644
>> >>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> >>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> >>> @@ -258,9 +258,11 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>> >>> if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
>> >>> ath_start_ani(common);
>> >>> ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
>> >>> - ath_beacon_config(sc, NULL);
>> >>> }
>> >>>
>> >>> + if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)))
>> >>> + ath_beacon_config(sc, NULL);
>> >>> +
>> >>
>> >> Upon further thought this change should just remove the SC_OP_SCANNING
>> >> flag from above and let the code there run when we are not off
>> >> channel. Reason is that as I noted above on the commit log entry the
>> >> scan flag will be set even when we return to the home channel due to
>> >> the races in mac80211 that were actually fixed by Johannes but later
>> >> reverted due to issues. Doing the fix this way would also let us
>> >> re-arm ANI and the tx completion monitor, this was also broken! I'll
>> >> respin this patch shortly after some basic testing.
>> >
>> > I should also note that this patch fixes the issue of reseting the
>> > beacon timers upon a reset, likely when the tx monitor hits -- I
>> > suspect that when that currently would hit we'd actually be out of
>> > synch with the AP and disconnect. This patch fixes that, but I just
>> > reviewed older kernels and the offchannel flag was not available prior
>> > to 2.6.36 so we can only fix this properly on >= 2.6.36 unless we
>> > figure out a way to annotate we're on the home channel. If we do we
>> > can potentially propagate a fix down to older kernels than 2.6.36
>> > eventually.
>>
>> Fun, just noticed these would be busted too :)
>>
>> ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
>>
>> Ah, joy.
>
> More joy, so I tried this as an alternative:
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 5fe570b..ecd6724 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -563,7 +563,6 @@ struct ath_ant_comb {
> #define SC_OP_RXFLUSH BIT(7)
> #define SC_OP_LED_ASSOCIATED BIT(8)
> #define SC_OP_LED_ON BIT(9)
> -#define SC_OP_SCANNING BIT(10)
> #define SC_OP_TSF_RESET BIT(11)
> #define SC_OP_BT_PRIORITY_DETECTED BIT(12)
> #define SC_OP_BT_SCAN BIT(13)
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index ba029b2..2207cf2 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -255,7 +255,7 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
> ath_update_txpow(sc);
> ath9k_hw_set_interrupts(ah, ah->imask);
>
> - if (!(sc->sc_flags & (SC_OP_OFFCHANNEL | SC_OP_SCANNING))) {
> + if (!(sc->sc_flags & SC_OP_OFFCHANNEL)) {
> ath_start_ani(common);
> ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
> ath_beacon_config(sc, NULL);
> @@ -957,7 +957,7 @@ int ath_reset(struct ath_softc *sc, bool retry_tx)
>
> ath_update_txpow(sc);
>
> - if (sc->sc_flags & SC_OP_BEACONS)
> + if ((sc->sc_flags & SC_OP_BEACONS) || !(sc->sc_flags & (SC_OP_OFFCHANNEL)))
> ath_beacon_config(sc, NULL); /* restart beacons */
>
> ath9k_hw_set_interrupts(ah, ah->imask);
> @@ -2042,7 +2042,6 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
>
> aphy->state = ATH_WIPHY_SCAN;
> ath9k_wiphy_pause_all_forced(sc, aphy);
> - sc->sc_flags |= SC_OP_SCANNING;
> mutex_unlock(&sc->mutex);
> }
>
> @@ -2057,7 +2056,6 @@ static void ath9k_sw_scan_complete(struct ieee80211_hw *hw)
>
> mutex_lock(&sc->mutex);
> aphy->state = ATH_WIPHY_ACTIVE;
> - sc->sc_flags &= ~SC_OP_SCANNING;
> mutex_unlock(&sc->mutex);
> }
>
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index 87e02f5..6f7d4d1 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -301,7 +301,7 @@ static void ath_edma_start_recv(struct ath_softc *sc)
>
> ath_opmode_init(sc);
>
> - ath9k_hw_startpcureceive(sc->sc_ah, (sc->sc_flags & SC_OP_SCANNING));
> + ath9k_hw_startpcureceive(sc->sc_ah, !!(sc->sc_flags & SC_OP_OFFCHANNEL));
> }
>
> static void ath_edma_stop_recv(struct ath_softc *sc)
> @@ -507,7 +507,7 @@ int ath_startrecv(struct ath_softc *sc)
> start_recv:
> spin_unlock_bh(&sc->rx.rxbuflock);
> ath_opmode_init(sc);
> - ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_SCANNING));
> + ath9k_hw_startpcureceive(ah, !!(sc->sc_flags & SC_OP_OFFCHANNEL));
>
> return 0;
> }
>
>
> But the harware goes nutty. Although technically it seems correct
I manged to figure out the issue, ANI needs to be started *after* the
beacon config. I'm going to split up these two fixes up separatey
though as one fixes the beacon loss as a regression, the other fixes a
long time issue of having left ANI completely disabled when doing a bg
scan.
Luis
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-14 22:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-14 15:00 [PATCH v2 02/10] ath9k: fix regression on beacon loss after bgscan Luis R. Rodriguez
2010-09-14 16:39 ` Luis R. Rodriguez
2010-09-14 16:42 ` Luis R. Rodriguez
2010-09-14 17:56 ` Luis R. Rodriguez
2010-09-14 20:06 ` Luis R. Rodriguez
2010-09-14 22:00 ` Luis R. Rodriguez
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.