2008-10-01 21:35:05

by Elias Oltmanns

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

Toralf F=F6rster <[email protected]> wrote:
> Hello,
>
> the issue (initially reported in August 2008) still remains in the cu=
rrent=20
> kernel at my ThinkPad T41, a screen shot is attached. The steps to re=
produce=20
> 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 separate
patches?

Regards,

Elias

---
=46rom: Elias Oltmanns <[email protected]>
Subject: ath5k: Do not start the hw unconditionally on resume from s2ra=
m

Currently, ath5k_pci_resume() calls ath5k_init() unconditionally.
Therefore, the following sequence of events leads to a kernel panic:
# modprobe ath5k
suspend to ram
resume
# modprobe -r ath5k
The calibration timer is not stopped even though the structs go away
underneath.

Signed-off-by: Elias Oltmanns <[email protected]>
---

drivers/net/wireless/ath5k/base.c | 8 +++++---
include/net/mac80211.h | 11 +++++++++++
net/mac80211/main.c | 6 ++++++
3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/a=
th5k/base.c
index 0676c6d..df602ee 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -621,9 +621,11 @@ ath5k_pci_resume(struct pci_dev *pdev)
goto err_no_irq;
}
=20
- err =3D ath5k_init(sc);
- if (err)
- goto err_irq;
+ if (ieee80211_opened(hw)) {
+ err =3D ath5k_init(sc);
+ if (err)
+ goto err_irq;
+ }
ath5k_led_enable(sc);
=20
/*
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ff137fd..d15a6ba 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1671,6 +1671,17 @@ void ieee80211_iterate_active_interfaces_atomic(=
struct ieee80211_hw *hw,
void *data);
=20
/**
+ * ieee80211_opened - Determine whether device has been opened.
+ * @hw: pointer as obtained from ieee80211_alloc_hw().
+ * @return: how many times interface has been opened
+ *
+ * Allows low level drivers to find out whether the ieee80211 layer
+ * considers the device opened. In fact, the number of opened virtual
+ * interfaces associated with the hw is returned.
+ */
+int ieee80211_opened(struct ieee80211_hw *hw);
+
+/**
* ieee80211_start_tx_ba_session - Start a tx Block Ack session.
* @hw: pointer as obtained from ieee80211_alloc_hw().
* @ra: receiver address of the BA session recipient
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index aa5a191..4f77496 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -557,6 +557,12 @@ static int ieee80211_stop(struct net_device *dev)
return 0;
}
=20
+int ieee80211_opened(struct ieee80211_hw *hw)
+{
+ return hw_to_local(hw)->open_count;
+}
+EXPORT_SYMBOL(ieee80211_opened);
+
int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16=
tid)
{
struct ieee80211_local *local =3D hw_to_local(hw);


2008-10-02 09:22:06

by Johannes Berg

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

On Wed, 2008-10-01 at 23:10 +0200, Elias Oltmanns wrote:

> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -557,6 +557,12 @@ static int ieee80211_stop(struct net_device *dev)
> return 0;
> }
>
> +int ieee80211_opened(struct ieee80211_hw *hw)
> +{
> + return hw_to_local(hw)->open_count;
> +}
> +EXPORT_SYMBOL(ieee80211_opened);

No way. The driver has all the info it needs. Fix it.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-01 22:15:13

by Bob Copeland

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

On Wed, Oct 1, 2008 at 5:10 PM, Elias Oltmanns <[email protected]> wrot=
e:
> Toralf F=F6rster <[email protected]> wrote:
>> Hello,
>>
>> the issue (initially reported in August 2008) still remains in the c=
urrent
>> kernel at my ThinkPad T41, a screen shot is attached. The steps to r=
eproduce
>> 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 separate
> 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 fro=
m
resume. Do you have a better idea of why calib_tim isn't cleaned up
properly by ath5k_stop_hw?

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

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