Re: [ath5k-devel] Oops with current kernel and ath5k

[Elias Oltmanns <eo@nebensachen.de>]

Bob Copeland <me@bobcopeland.com> wrote:
> On Tue, Oct 07, 2008 at 12:44:58PM +0200, Elias Oltmanns wrote:
>> Bob Copeland <me@bobcopeland.com> wrote:
>> > -ath5k_stop_hw(struct ath5k_softc *sc)
>> > +ath5k_stop_hw(struct ath5k_softc *sc, bool update_status)
>
> Now with more git-add action...
>
> Subject: [PATCH] ath5k: correct hardware startup sequence in resume
>
> 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.
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---
>  drivers/net/wireless/ath5k/base.c |   28 ++++++++++++++++++++--------
>  drivers/net/wireless/ath5k/base.h |    3 ++-
>  2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index c151588..ae1c152 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
[...]
> @@ -2280,6 +2289,9 @@ ath5k_stop_hw(struct ath5k_softc *sc)
>  	}
>  	ath5k_txbuf_free(sc, sc->bbuf);
>  	mmiowb();
> +
> +	if (!is_suspend)
> +		__clear_bit(ATH_STAT_STARTED, sc->status);
>  	mutex_unlock(&sc->lock);

Personally, I'd prefer keeping the memory barrier right next to the
mutex_unlock(), i.e.

 	ath5k_txbuf_free(sc, sc->bbuf);
+	if (!is_suspend)
+		__clear_bit(ATH_STAT_STARTED, sc->status);
+
 	mmiowb();
 	mutex_unlock(&sc->lock);

even though this is purely a matter of style in this particular case.
Otherwise:

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