Re: [patch 27/27] ath5k: fix suspend-related oops on rmmod

[Elias Oltmanns <eo@nebensachen.de>]

Greg KH <gregkh@suse.de> wrote:
> 2.6.27-stable review patch.  If anyone has any objections, please let us 
> know.
>
> ------------------
> From: Bob Copeland <me@bobcopeland.com>
>
> commit 8bdd5b9c6bd53add260756b6673a0545fbdbba21 upstream
>
> Based on a patch by Elias Oltmanns, we call ath5k_init in resume even
> if we didn't previously open the device.  Besides starting up the
> device unnecessarily, this also causes an oops on rmmod because
> mac80211 will not invoke ath5k_stop and softirqs are left running after
> the module has been unloaded.  Add a new state bit, ATH_STAT_STARTED,
> to indicate that we have been started up.
>
> Reported-by: Toralf Förster <toralf.foerster@gmx.de>
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

I've done some more testing and I'm ashamed to say that this patch is
incomplete and does indeed introduce a regression itself. Having only
just sent the fix to John, I'm afraid this patch has to be deferred for
the next stable release since the required fix is not likely to hit
Linus' tree in time. Anyway, I'm adding my patch and the changelog entry
by way of explanation.

Regards,

Elias

From: Elias Oltmanns <eo@nebensachen.de>
Subject: [PATCH] ath5k: Reset key cache on interface up, thus fixing resume

After a s2ram / resume cycle, resetting the key cache does not work
unless it is deferred until after the hardware has been reinitialised by
a call to ath5k_hw_reset(). This fixes a regression introduced in commit
8bdd5b9c6bd53add260756b6673a0545fbdbba21 (ath5k: fix suspend-related
oops on rmmod).

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 base.c |   29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 0f1d6bd..8acf0ce 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -688,16 +688,6 @@ ath5k_pci_resume(struct pci_dev *pdev)
 		goto err_irq;
 	ath5k_led_enable(sc);
 
-	/*
-	 * Reset the key cache since some parts do not
-	 * reset the contents on initial power up or resume.
-	 *
-	 * FIXME: This may need to be revisited when mac80211 becomes
-	 *        aware of suspend/resume.
-	 */
-	for (i = 0; i < AR5K_KEYTABLE_SIZE; i++)
-		ath5k_hw_reset_key(ah, i);
-
 	return 0;
 err_irq:
 	free_irq(pdev->irq, sc);
@@ -737,13 +727,6 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw)
 		__set_bit(ATH_STAT_MRRETRY, sc->status);
 
 	/*
-	 * Reset the key cache since some parts do not
-	 * reset the contents on initial power up.
-	 */
-	for (i = 0; i < AR5K_KEYTABLE_SIZE; i++)
-		ath5k_hw_reset_key(ah, i);
-
-	/*
 	 * Collect the channel list.  The 802.11 layer
 	 * is resposible for filtering this list based
 	 * on settings like the phy mode and regulatory
@@ -2202,7 +2185,8 @@ ath5k_beacon_config(struct ath5k_softc *sc)
 static int
 ath5k_init(struct ath5k_softc *sc, bool is_resume)
 {
-	int ret;
+	struct ath5k_hw *ah = sc->ah;
+	int ret, i;
 
 	mutex_lock(&sc->lock);
 
@@ -2235,10 +2219,17 @@ ath5k_init(struct ath5k_softc *sc, bool is_resume)
 	if (ret)
 		goto done;
 
+	/*
+	 * Reset the key cache since some parts do not reset the
+	 * contents on initial power up or resume from suspend.
+	 */
+	for (i = 0; i < AR5K_KEYTABLE_SIZE; i++)
+		ath5k_hw_reset_key(ah, i);
+
 	__set_bit(ATH_STAT_STARTED, sc->status);
 
 	/* Set ack to be sent at low bit-rates */
-	ath5k_hw_set_ack_bitrate_high(sc->ah, false);
+	ath5k_hw_set_ack_bitrate_high(ah, false);
 
 	mod_timer(&sc->calib_tim, round_jiffies(jiffies +
 			msecs_to_jiffies(ath5k_calinterval * 1000)));