2008-10-01 22:34:49

by Elias Oltmanns

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

"Bob Copeland" <[email protected]> wrote:
> On Wed, Oct 1, 2008 at 5:10 PM, Elias Oltmanns <[email protected]> wr=
ote:
>> Toralf F=F6rster <[email protected]> wrote:
>
>>> Hello,
>>>
>>> the issue (initially reported in August 2008) still remains in the =
current
>>> kernel at my ThinkPad T41, a screen shot is attached. The steps to =
reproduce
>>> are :
>>>
>>> 1. modprobe it
>>> 2. suspend the system to ram
>>> 3. wake it up
>>> 4. rmmod the driver
>>
>> Yes, I have run into this problem too. The patch below (applies to
>> 2.6.27-rc8) fixes the problem, but since I'm not a wireless hacker,
>> developers might prefer a different approach. Please let me know if =
I
>> should change anything. Perhaps I should split this into two separat=
e
>> patches?
>
> Thanks for the patch. I do think a different approach is probably
> warranted, though. ath5k_init() is supposed to be able to be called f=
rom
> resume. Do you have a better idea of why calib_tim isn't cleaned up
> properly by ath5k_stop_hw?

Oh, but it is cleaned up by ath5k_stop_hw(). The issue is a different
one here:

ath5k_init() =3D=3D ->start()
ath5k_stop_hw() =3D=3D ->stop()

Since the mac80211 layer never opened a device, it won't close it
either. Thus, ath5k_stop_hw() does not get called on module unload. By
calling ath5k_init() on resume, the driver has effectively started the
device when it was not supposed to do so and this event is not being
tracked by the higher layers.

>
> I'm not near my laptop to test -- does the issue ever happen if you d=
o
> NOT suspend?

If you don't do a suspend / resume cycle but simply load and unload the
module, this is not an issue because ath5k_init() never gets called
unless the mac80211 layer tells the driver to do so.

Regards,

Elias


2008-10-02 02:04:44

by Bob Copeland

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

On Wed, Oct 1, 2008 at 6:34 PM, Elias Oltmanns <[email protected]> wrote:
> "Bob Copeland" <[email protected]> wrote:
>> On Wed, Oct 1, 2008 at 5:10 PM, Elias Oltmanns <[email protected]> wrote:
> Oh, but it is cleaned up by ath5k_stop_hw(). The issue is a different
> one here:
>
> ath5k_init() == ->start()
> ath5k_stop_hw() == ->stop()
>
> Since the mac80211 layer never opened a device, it won't close it
> either. Thus, ath5k_stop_hw() does not get called on module unload. By
> calling ath5k_init() on resume, the driver has effectively started the
> device when it was not supposed to do so and this event is not being
> tracked by the higher layers.

Ah, yes, good analysis... and ath5k_init is scheduling the timer. I
doubt ieee80211_opened would fly; probably the better thing to do is
ensure that the cleanup gets called regardless of whether ath5k_stop
gets called (it shouldn't matter since the irqs all get created in
_attach).

--
Bob Copeland %% http://www.bobcopeland.com