[PATCH 0/3] ath9k: move mac80211 workqueue cancelations

[PATCH 0/3] ath9k: move mac80211 workqueue cancelations

[Luis R. Rodriguez]

This corrects the ath9k cancelations of work for the mac80211 workqueue
on the stop callback.

Luis R. Rodriguez (3):
  ath9k: re-order cancelling of work on mac80211 workqueue
  ath9k: move cancel_delayed_work_sync() out of ath_deinit_leds()
  ath9k: move workqueue cancels to stop callback

 drivers/net/wireless/ath/ath9k/main.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

[PATCH 1/3] ath9k: re-order cancelling of work on mac80211 workqueue

[Luis R. Rodriguez]

ath9k uses the mac80211 workqueue for 4 different types of work:

 * Led blink work
 * TX hang monitoring work
 * internal wiphy schedular work
 * channel change work done for internal wiphy schedular

Since the internal wiphy schedular can end up kicking off some
channel channel change work we should first cancel the wiphy
schedular work and then the channel change work.

The TX hang work can be cancelled second since we're going down
anyway.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 1ce1de5..dfcd8d9 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1253,9 +1253,9 @@ void ath_detach(struct ath_softc *sc)
 	DPRINTF(sc, ATH_DBG_CONFIG, "Detach ATH hw\n");
 
 	ath_deinit_leds(sc);
-	cancel_work_sync(&sc->chan_work);
-	cancel_delayed_work_sync(&sc->wiphy_work);
 	cancel_delayed_work_sync(&sc->tx_complete_work);
+	cancel_delayed_work_sync(&sc->wiphy_work);
+	cancel_work_sync(&sc->chan_work);
 
 	for (i = 0; i < sc->num_sec_wiphy; i++) {
 		struct ath_wiphy *aphy = sc->sec_wiphy[i];
-- 
1.6.0.4

[PATCH 2/3] ath9k: move cancel_delayed_work_sync() out of ath_deinit_leds()

[Luis R. Rodriguez]

We do this as we'll be moving the cancel elsewhere later.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 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 dfcd8d9..dbd5cfd 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1057,7 +1057,6 @@ static void ath_unregister_led(struct ath_led *led)
 
 static void ath_deinit_leds(struct ath_softc *sc)
 {
-	cancel_delayed_work_sync(&sc->ath_led_blink_work);
 	ath_unregister_led(&sc->assoc_led);
 	sc->sc_flags &= ~SC_OP_LED_ASSOCIATED;
 	ath_unregister_led(&sc->tx_led);
@@ -1114,6 +1113,7 @@ static void ath_init_leds(struct ath_softc *sc)
 	return;
 
 fail:
+	cancel_delayed_work_sync(&sc->ath_led_blink_work);
 	ath_deinit_leds(sc);
 }
 
@@ -1252,11 +1252,13 @@ void ath_detach(struct ath_softc *sc)
 
 	DPRINTF(sc, ATH_DBG_CONFIG, "Detach ATH hw\n");
 
-	ath_deinit_leds(sc);
+	cancel_delayed_work_sync(&sc->ath_led_blink_work);
 	cancel_delayed_work_sync(&sc->tx_complete_work);
 	cancel_delayed_work_sync(&sc->wiphy_work);
 	cancel_work_sync(&sc->chan_work);
 
+	ath_deinit_leds(sc);
+
 	for (i = 0; i < sc->num_sec_wiphy; i++) {
 		struct ath_wiphy *aphy = sc->sec_wiphy[i];
 		if (aphy == NULL)
-- 
1.6.0.4

[PATCH 3/3] ath9k: move workqueue cancels to stop callback

[Luis R. Rodriguez]

We should be cancelling our work at the stop callback since
we are borrowing the mac80211 workqueue for our work. As it
stands mac80211 expects this for suspend purposes.

The ath9k specific virtual wiphy stuff need only be
cancelled only when the we have no secondary virtual wiphys.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index dbd5cfd..50d99d7 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1252,11 +1252,6 @@ void ath_detach(struct ath_softc *sc)
 
 	DPRINTF(sc, ATH_DBG_CONFIG, "Detach ATH hw\n");
 
-	cancel_delayed_work_sync(&sc->ath_led_blink_work);
-	cancel_delayed_work_sync(&sc->tx_complete_work);
-	cancel_delayed_work_sync(&sc->wiphy_work);
-	cancel_work_sync(&sc->chan_work);
-
 	ath_deinit_leds(sc);
 
 	for (i = 0; i < sc->num_sec_wiphy; i++) {
@@ -2092,6 +2087,14 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 
 	aphy->state = ATH_WIPHY_INACTIVE;
 
+	cancel_delayed_work_sync(&sc->ath_led_blink_work);
+	cancel_delayed_work_sync(&sc->tx_complete_work);
+
+	if (!sc->num_sec_wiphy) {
+		cancel_delayed_work_sync(&sc->wiphy_work);
+		cancel_work_sync(&sc->chan_work);
+	}
+
 	if (sc->sc_flags & SC_OP_INVALID) {
 		DPRINTF(sc, ATH_DBG_ANY, "Device not present\n");
 		return;
-- 
1.6.0.4

Re: [PATCH 3/3] ath9k: move workqueue cancels to stop callback

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Mon, 2009-07-27 at 11:53 -0700, Luis R. Rodriguez wrote:
> We should be cancelling our work at the stop callback since
> we are borrowing the mac80211 workqueue for our work. As it
> stands mac80211 expects this for suspend purposes.
> 
> The ath9k specific virtual wiphy stuff need only be
> cancelled only when the we have no secondary virtual wiphys.

inverted logic here?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 3/3] ath9k: move workqueue cancels to stop callback

[Luis R. Rodriguez]

On Mon, Jul 27, 2009 at 12:10 PM, Johannes
Berg<johannes@sipsolutions.net> wrote:
> On Mon, 2009-07-27 at 11:53 -0700, Luis R. Rodriguez wrote:
>> We should be cancelling our work at the stop callback since
>> we are borrowing the mac80211 workqueue for our work. As it
>> stands mac80211 expects this for suspend purposes.
>>
>> The ath9k specific virtual wiphy stuff need only be
>> cancelled only when the we have no secondary virtual wiphys.
>
> inverted logic here?

Nope, ie the sc->num_sec_wiphy wil be decremented on device removal.
As long as there is some secondary wiphy it means we need our internal
scheduler working and work is being queued. So upon suspend we'll
iterate over all interfaces and remove them unless you have WoW
enabled (eventually), the last interface drv_stop() then would ensure
to clear the virtual scheduler work.

  Luis