When signal is noisy, hardware can use all RX buffers and since the las=
t
entry in the list is self-linked, it overwrites the entry until we link
new buffers.
Ensure that we don't free this last one until we are 100% sure that it
is not used by the hardware anymore to not cause memory curruption as
can be seen below.
This is done by checking next buffer in the list. Even after that we
know that the hardware refetched the new link and proceeded further
(the next buffer is ready) we can finally free the overwritten buffer.
We discard it since the status in its descriptor is overwritten (OR-ed
by new status) too.
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
BUG kmalloc-4096: Poison overwritten
-----------------------------------------------------------------------=
------
INFO: 0xffff810067419060-0xffff810067419667. First byte 0x8 instead of =
0x6b
INFO: Allocated in dev_alloc_skb+0x18/0x30 age=3D1118 cpu=3D1 pid=3D0
INFO: Freed in skb_release_data+0x85/0xd0 age=3D1105 cpu=3D1 pid=3D3718
INFO: Slab 0xffffe200019d0600 objects=3D7 used=3D0 fp=3D0xffff810067419=
048 flags=3D0x40000000000020c3
INFO: Object 0xffff810067419048 @offset=3D4168 fp=3D0xffff81006741c120
Bytes b4 0xffff810067419038: 4f 0b 02 00 01 00 00 00 5a 5a 5a 5a 5a 5a=
5a 5a O.......ZZZZZZZZ
Object 0xffff810067419048: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b=
6b 6b kkkkkkkkkkkkkkkk
Object 0xffff810067419058: 6b 6b 6b 6b 6b 6b 6b 6b 08 42 30 00 00 0b=
6b 80 kkkkkkkk.B0...k.
Object 0xffff810067419068: f0 5d 00 4f 62 08 a3 64 00 0c 42 16 52 e4=
f0 5a 360].Ob.243d..B.R344360Z
Object 0xffff810067419078: 68 81 00 00 7b a5 b4 be 7d 3b 8f 53 cd d5=
de 12 h...{245264276};.S315325336.
Object 0xffff810067419088: 96 10 0b 89 48 54 23 41 0f 4e 2d b9 37 c3=
cb 29 ....HT#A.N-2717303313)
Object 0xffff810067419098: d1 e0 de 14 8a 57 2a cc 3b 44 0d 78 7a 19=
12 15 321340336..W*314;D.xz...
Object 0xffff8100674190a8: a9 ec d4 35 a8 10 ec 8c 40 a7 06 0a 51 a7=
48 bb [email protected]
Object 0xffff8100674190b8: 3e cf a1 c7 38 60 63 3f 51 15 c7 20 eb ba=
65 30 >=CF=A13078`c?Q.307.353272e0
Redzone 0xffff81006741a048: bb bb bb bb bb bb bb bb =
273273273273273273273273
Padding 0xffff81006741a088: 5a 5a 5a 5a 5a 5a 5a 5a =
ZZZZZZZZ
Pid: 3297, comm: ath5k_pci Not tainted 2.6.26-rc8-mm1_64 #427
Call Trace:
[<ffffffff802a7306>] print_trailer+0xf6/0x150
[<ffffffff802a7485>] check_bytes_and_report+0x125/0x180
[<ffffffff802a75dc>] check_object+0xac/0x260
[<ffffffff802a9308>] __slab_alloc+0x368/0x6d0
[<ffffffff80544f82>] ? wireless_send_event+0x142/0x310
[<ffffffff804b1bd4>] ? __alloc_skb+0x44/0x150
[<ffffffff80544f82>] ? wireless_send_event+0x142/0x310
[<ffffffff802aa853>] __kmalloc_track_caller+0xc3/0xf0
[<ffffffff804b1bfe>] __alloc_skb+0x6e/0x150
[... stack snipped]
=46IX kmalloc-4096: Restoring 0xffff810067419060-0xffff810067419667=3D0=
x6b
=46IX kmalloc-4096: Marking all objects used
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 32 +++++++++++++++++++++++++----=
---
drivers/net/wireless/ath5k/base.h | 2 +-
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/a=
th5k/base.c
index 12a9443..e9ec284 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1683,20 +1683,21 @@ ath5k_tasklet_rx(unsigned long data)
struct ath5k_rx_status rs =3D {};
struct sk_buff *skb;
struct ath5k_softc *sc =3D (void *)data;
- struct ath5k_buf *bf;
+ struct ath5k_buf *bf, *bf_last;
struct ath5k_desc *ds;
int ret;
int hdrlen;
int pad;
=20
spin_lock(&sc->rxbuflock);
+ if (list_empty(&sc->rxbuf)) {
+ ATH5K_WARN(sc, "empty rx buf pool\n");
+ goto unlock;
+ }
+ bf_last =3D list_entry(sc->rxbuf.prev, struct ath5k_buf, list);
do {
rxs.flag =3D 0;
=20
- if (unlikely(list_empty(&sc->rxbuf))) {
- ATH5K_WARN(sc, "empty rx buf pool\n");
- break;
- }
bf =3D list_first_entry(&sc->rxbuf, struct ath5k_buf, list);
BUG_ON(bf->skb =3D=3D NULL);
skb =3D bf->skb;
@@ -1706,8 +1707,24 @@ ath5k_tasklet_rx(unsigned long data)
pci_dma_sync_single_for_cpu(sc->pdev, sc->desc_daddr,
sc->desc_len, PCI_DMA_FROMDEVICE);
=20
- if (unlikely(ds->ds_link =3D=3D bf->daddr)) /* this is the end */
- break;
+ /*
+ * last buffer must not be freed to ensure proper hardware
+ * function. When the hardware finishes also a packet next to
+ * it, we are sure, it doesn't use it anymore and we can go on.
+ */
+ if (bf_last =3D=3D bf)
+ bf->flags |=3D 1;
+ if (bf->flags) {
+ struct ath5k_buf *bf_next =3D list_entry(bf->list.next,
+ struct ath5k_buf, list);
+ ret =3D sc->ah->ah_proc_rx_desc(sc->ah, bf_next->desc,
+ &rs);
+ if (ret)
+ break;
+ bf->flags &=3D ~1;
+ /* skip the overwritten one (even status is martian) */
+ goto next;
+ }
=20
ret =3D sc->ah->ah_proc_rx_desc(sc->ah, ds, &rs);
if (unlikely(ret =3D=3D -EINPROGRESS))
@@ -1817,6 +1834,7 @@ accept:
next:
list_move_tail(&bf->list, &sc->rxbuf);
} while (ath5k_rxbuf_setup(sc, bf) =3D=3D 0);
+unlock:
spin_unlock(&sc->rxbuflock);
}
=20
diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/a=
th5k/base.h
index 47f414b..d7e03e6 100644
--- a/drivers/net/wireless/ath5k/base.h
+++ b/drivers/net/wireless/ath5k/base.h
@@ -56,7 +56,7 @@
=20
struct ath5k_buf {
struct list_head list;
- unsigned int flags; /* tx descriptor flags */
+ unsigned int flags; /* rx descriptor flags */
struct ath5k_desc *desc; /* virtual addr of desc */
dma_addr_t daddr; /* physical addr of desc */
struct sk_buff *skb; /* skbuff for buf */
--=20
1.5.6.2
2008/7/15 Jiri Slaby <[email protected]>:
> Make sure that the irq is not in progress after stop. This means
> two things:
> - ensure the intr setting register is set by flushing posted values
> - call synchronize_irq() after that
>
> Also flush stop tx write, inform callers of the tx stop about still
> pending transfers (unsuccessful stop) and finally don't wait another
> 3ms in ath5k_rx_stop, since ath5k_hw_stop_rx_dma ensures transfer to
> be finished.
>
> Make sure all writes will be ordered in respect to locks by mmiowb().
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Luis R. Rodriguez <[email protected]>
> ---
> drivers/net/wireless/ath5k/base.c | 13 +++++++++++--
> drivers/net/wireless/ath5k/hw.c | 4 ++++
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 4d9ff97..4874a6f 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -43,7 +43,9 @@
> #include <linux/version.h>
> #include <linux/module.h>
> #include <linux/delay.h>
> +#include <linux/hardirq.h>
> #include <linux/if.h>
> +#include <linux/io.h>
> #include <linux/netdevice.h>
> #include <linux/cache.h>
> #include <linux/pci.h>
> @@ -1250,6 +1252,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf)
>
> txq->link = &ds->ds_link;
> ath5k_hw_tx_start(ah, txq->qnum);
> + mmiowb();
> spin_unlock_bh(&txq->lock);
>
> return 0;
> @@ -1584,7 +1587,6 @@ ath5k_rx_stop(struct ath5k_softc *sc)
> ath5k_hw_stop_pcu_recv(ah); /* disable PCU */
> ath5k_hw_set_rx_filter(ah, 0); /* clear recv filter */
> ath5k_hw_stop_rx_dma(ah); /* disable DMA engine */
> - mdelay(3); /* 3ms is long enough for 1 frame */
>
> ath5k_debug_printrxbuffs(sc, ah);
>
> @@ -2259,6 +2261,7 @@ ath5k_init(struct ath5k_softc *sc)
>
> ret = 0;
> done:
> + mmiowb();
> mutex_unlock(&sc->lock);
> return ret;
> }
> @@ -2291,6 +2294,7 @@ ath5k_stop_locked(struct ath5k_softc *sc)
> if (!test_bit(ATH_STAT_INVALID, sc->status)) {
> ath5k_led_off(sc);
> ath5k_hw_set_intr(ah, 0);
> + synchronize_irq(sc->pdev->irq);
> }
> ath5k_txq_cleanup(sc);
> if (!test_bit(ATH_STAT_INVALID, sc->status)) {
> @@ -2340,6 +2344,7 @@ ath5k_stop_hw(struct ath5k_softc *sc)
> }
> }
> ath5k_txbuf_free(sc, sc->bbuf);
> + mmiowb();
> mutex_unlock(&sc->lock);
>
> del_timer_sync(&sc->calib_tim);
> @@ -2805,6 +2810,7 @@ ath5k_config_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> /* XXX: assoc id is set to 0 for now, mac80211 doesn't have
> * a clean way of letting us retrieve this yet. */
> ath5k_hw_set_associd(ah, ah->ah_bssid, 0);
> + mmiowb();
> }
> mutex_unlock(&sc->lock);
>
> @@ -2981,6 +2987,7 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> }
>
> unlock:
> + mmiowb();
> mutex_unlock(&sc->lock);
> return ret;
> }
> @@ -3054,8 +3061,10 @@ ath5k_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb)
> ret = ath5k_beacon_setup(sc, sc->bbuf);
> if (ret)
> sc->bbuf->skb = NULL;
> - else
> + else {
> ath5k_beacon_config(sc);
> + mmiowb();
> + }
>
> end:
> mutex_unlock(&sc->lock);
> diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
> index c6d12c5..7ca87a5 100644
> --- a/drivers/net/wireless/ath5k/hw.c
> +++ b/drivers/net/wireless/ath5k/hw.c
> @@ -1440,6 +1440,7 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
>
> /* Stop queue */
> ath5k_hw_reg_write(ah, tx_queue, AR5K_CR);
> + ath5k_hw_reg_read(ah, AR5K_CR);
> } else {
> /*
> * Schedule TX disable and wait until queue is empty
> @@ -1456,6 +1457,8 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
>
> /* Clear register */
> ath5k_hw_reg_write(ah, 0, AR5K_QCU_TXD);
> + if (pending)
> + return -EBUSY;
> }
>
> /* TODO: Check for success else return error */
> @@ -1716,6 +1719,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)
>
> /* ..re-enable interrupts */
> ath5k_hw_reg_write(ah, AR5K_IER_ENABLE, AR5K_IER);
> + ath5k_hw_reg_read(ah, AR5K_IER);
>
> return old_mask;
> }
> --
> 1.5.6.2
>
>
Acked-by: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
Don't forget to kill tasklets on stop to not panic if they
fire after freeing some structures.
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index e9ec284..4d9ff97 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -2343,6 +2343,9 @@ ath5k_stop_hw(struct ath5k_softc *sc)
mutex_unlock(&sc->lock);
del_timer_sync(&sc->calib_tim);
+ tasklet_kill(&sc->rxtq);
+ tasklet_kill(&sc->txtq);
+ tasklet_kill(&sc->restq);
return ret;
}
--
1.5.6.2
- free and re-request irq since it might have changed during suspend
- disable and enable msi
- don't set D0 state of the device, it's already done by the PCI layer
- do restore_state before enable_device, it's safer
- check ath5k_init return value
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
Cc: Jesse Barnes <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 26 +++++++++++++++++++++-----
1 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 713ee99..d58a883 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -593,6 +593,9 @@ ath5k_pci_suspend(struct pci_dev *pdev, pm_message_t state)
ath5k_led_off(sc);
ath5k_stop_hw(sc);
+
+ free_irq(pdev->irq, sc);
+ pci_disable_msi(pdev);
pci_save_state(pdev);
pci_disable_device(pdev);
pci_set_power_state(pdev, PCI_D3hot);
@@ -608,15 +611,12 @@ ath5k_pci_resume(struct pci_dev *pdev)
struct ath5k_hw *ah = sc->ah;
int i, err;
- err = pci_set_power_state(pdev, PCI_D0);
- if (err)
- return err;
+ pci_restore_state(pdev);
err = pci_enable_device(pdev);
if (err)
return err;
- pci_restore_state(pdev);
/*
* Suspend/Resume resets the PCI configuration space, so we have to
* re-disable the RETRY_TIMEOUT register (0x41) to keep
@@ -624,7 +624,17 @@ ath5k_pci_resume(struct pci_dev *pdev)
*/
pci_write_config_byte(pdev, 0x41, 0);
- ath5k_init(sc);
+ pci_enable_msi(pdev);
+
+ err = request_irq(pdev->irq, ath5k_intr, IRQF_SHARED, "ath", sc);
+ if (err) {
+ ATH5K_ERR(sc, "request_irq failed\n");
+ goto err_msi;
+ }
+
+ err = ath5k_init(sc);
+ if (err)
+ goto err_irq;
ath5k_led_enable(sc);
/*
@@ -638,6 +648,12 @@ ath5k_pci_resume(struct pci_dev *pdev)
ath5k_hw_reset_key(ah, i);
return 0;
+err_irq:
+ free_irq(pdev->irq, sc);
+err_msi:
+ pci_disable_msi(pdev);
+ pci_disable_device(pdev);
+ return err;
}
#endif /* CONFIG_PM */
--
1.5.6.2
2008/7/15 Jiri Slaby <[email protected]>:
> Don't forget to kill tasklets on stop to not panic if they
> fire after freeing some structures.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Luis R. Rodriguez <[email protected]>
> ---
> drivers/net/wireless/ath5k/base.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index e9ec284..4d9ff97 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -2343,6 +2343,9 @@ ath5k_stop_hw(struct ath5k_softc *sc)
> mutex_unlock(&sc->lock);
>
> del_timer_sync(&sc->calib_tim);
> + tasklet_kill(&sc->rxtq);
> + tasklet_kill(&sc->txtq);
> + tasklet_kill(&sc->restq);
>
> return ret;
> }
> --
> 1.5.6.2
>
>
Acked-by: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On Wed, 2008-07-16 at 09:31 -0700, Jesse Barnes wrote:
> On Wednesday, July 16, 2008 9:15 am Nick Kossifidis wrote:
> > It's ok for now, but have in mind that on my upcoming patch series i'm
> > disabling msi (commented out) since it results no interrupts on pci-e
> > cards (seems there is a bug in kernel's msi implementation).
>
> Hm, would be good to get details here. MSI is being used by other drivers
> successfully...
That's true, but no driver uses the same interrupt handler with and
without MSI. Either it's different handlers or the handler checks if
MSI is enabled and does something differently.
Checks is any interrupts are pending are different for MSI. It may be
not hard to do (it's probably easier than for real interrupts), but it
has to be figured out. Somebody has to do it right. Until MSI is
supported by the interrupt handler, it should not be enabled by the
driver, or we get a non-functioning driver for AR5006.
I see that MSI is enabled in wireless-testing, but I think we should try
to avoid this mistake when the commit goes upstream. I'm not sure what
the procedure there, but it would be nice to keep the tree bisectable,
i.e. avoid known regressions in any revision. Maybe an amended commit
should go upstream, and we'll drop the original patch when rebasing to
the upstream branch?
--
Regards,
Pavel Roskin
On Wednesday, July 16, 2008 10:35 am Pavel Roskin wrote:
> On Wed, 2008-07-16 at 09:31 -0700, Jesse Barnes wrote:
> > On Wednesday, July 16, 2008 9:15 am Nick Kossifidis wrote:
> > > It's ok for now, but have in mind that on my upcoming patch series i'm
> > > disabling msi (commented out) since it results no interrupts on pci-e
> > > cards (seems there is a bug in kernel's msi implementation).
> >
> > Hm, would be good to get details here. MSI is being used by other
> > drivers successfully...
>
> That's true, but no driver uses the same interrupt handler with and
> without MSI. Either it's different handlers or the handler checks if
> MSI is enabled and does something differently.
Yeah, using MSI on your device can mean that you have to make some changes
(like on Intel graphics you have to disable regular interrupts or bad things
happen), but you can still share the handler or at least most of it in some
cases.
> Checks is any interrupts are pending are different for MSI. It may be
> not hard to do (it's probably easier than for real interrupts), but it
> has to be figured out. Somebody has to do it right. Until MSI is
> supported by the interrupt handler, it should not be enabled by the
> driver, or we get a non-functioning driver for AR5006.
Sure, that's fine. I just wanted to make sure that there weren't some weird
generic MSI problems.
Thanks,
Jesse
On Wednesday, July 16, 2008 12:27 pm Nick Kossifidis wrote:
> 2008/7/16 Jesse Barnes <[email protected]>:
> > On Wednesday, July 16, 2008 10:35 am Pavel Roskin wrote:
> >> On Wed, 2008-07-16 at 09:31 -0700, Jesse Barnes wrote:
> >> > On Wednesday, July 16, 2008 9:15 am Nick Kossifidis wrote:
> >> > > It's ok for now, but have in mind that on my upcoming patch series
> >> > > i'm disabling msi (commented out) since it results no interrupts on
> >> > > pci-e cards (seems there is a bug in kernel's msi implementation).
> >> >
> >> > Hm, would be good to get details here. MSI is being used by other
> >> > drivers successfully...
> >>
> >> That's true, but no driver uses the same interrupt handler with and
> >> without MSI. Either it's different handlers or the handler checks if
> >> MSI is enabled and does something differently.
> >
> > Yeah, using MSI on your device can mean that you have to make some
> > changes (like on Intel graphics you have to disable regular interrupts or
> > bad things happen), but you can still share the handler or at least most
> > of it in some cases.
>
> Is there documentation for this somewhere ? Msi documentation
> (MSI-HOWTO.txt) doesn't say anything about a different interrupt
> handler etc.
Matthew just rewrote MSI-HOWTO as part of his rework of the MSI code. It
might be more helpful now (see
http://kerneltrap.org/mailarchive/linux-kernel/2008/7/1).
>
> >> Checks is any interrupts are pending are different for MSI. It may be
> >> not hard to do (it's probably easier than for real interrupts), but it
> >> has to be figured out. Somebody has to do it right. Until MSI is
> >> supported by the interrupt handler, it should not be enabled by the
> >> driver, or we get a non-functioning driver for AR5006.
> >
> > Sure, that's fine. I just wanted to make sure that there weren't some
> > weird generic MSI problems.
>
> I was referring to this post...
> http://lkml.org/lkml/2008/6/24/150
Yeah, that's a real bug on x86; I don't know how widespread its effects are
though. I'll ping Ingo & Thomas again.
Jesse
Don't sync
- coherent mapping (descriptors)
- before unmap, it's useless
- (wrongly anyway -- for_cpu) beacon skb, it's just mapped,
so by the device yet
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 4874a6f..713ee99 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1705,10 +1705,6 @@ ath5k_tasklet_rx(unsigned long data)
skb = bf->skb;
ds = bf->desc;
- /* TODO only one segment */
- pci_dma_sync_single_for_cpu(sc->pdev, sc->desc_daddr,
- sc->desc_len, PCI_DMA_FROMDEVICE);
-
/*
* last buffer must not be freed to ensure proper hardware
* function. When the hardware finishes also a packet next to
@@ -1772,8 +1768,6 @@ ath5k_tasklet_rx(unsigned long data)
goto next;
}
accept:
- pci_dma_sync_single_for_cpu(sc->pdev, bf->skbaddr,
- rs.rs_datalen, PCI_DMA_FROMDEVICE);
pci_unmap_single(sc->pdev, bf->skbaddr, sc->rxbufsize,
PCI_DMA_FROMDEVICE);
bf->skb = NULL;
@@ -1861,9 +1855,6 @@ ath5k_tx_processq(struct ath5k_softc *sc, struct ath5k_txq *txq)
list_for_each_entry_safe(bf, bf0, &txq->q, list) {
ds = bf->desc;
- /* TODO only one segment */
- pci_dma_sync_single_for_cpu(sc->pdev, sc->desc_daddr,
- sc->desc_len, PCI_DMA_FROMDEVICE);
ret = sc->ah->ah_proc_tx_desc(sc->ah, ds, &ts);
if (unlikely(ret == -EINPROGRESS))
break;
@@ -2036,8 +2027,6 @@ ath5k_beacon_send(struct ath5k_softc *sc)
ATH5K_WARN(sc, "beacon queue %u didn't stop?\n", sc->bhalq);
/* NB: hw still stops DMA, so proceed */
}
- pci_dma_sync_single_for_cpu(sc->pdev, bf->skbaddr, bf->skb->len,
- PCI_DMA_TODEVICE);
ath5k_hw_put_tx_buf(ah, sc->bhalq, bf->daddr);
ath5k_hw_tx_start(ah, sc->bhalq);
--
1.5.6.2
2008/7/16 Jesse Barnes <[email protected]>:
> On Wednesday, July 16, 2008 10:35 am Pavel Roskin wrote:
>> On Wed, 2008-07-16 at 09:31 -0700, Jesse Barnes wrote:
>> > On Wednesday, July 16, 2008 9:15 am Nick Kossifidis wrote:
>> > > It's ok for now, but have in mind that on my upcoming patch series i'm
>> > > disabling msi (commented out) since it results no interrupts on pci-e
>> > > cards (seems there is a bug in kernel's msi implementation).
>> >
>> > Hm, would be good to get details here. MSI is being used by other
>> > drivers successfully...
>>
>> That's true, but no driver uses the same interrupt handler with and
>> without MSI. Either it's different handlers or the handler checks if
>> MSI is enabled and does something differently.
>
> Yeah, using MSI on your device can mean that you have to make some changes
> (like on Intel graphics you have to disable regular interrupts or bad things
> happen), but you can still share the handler or at least most of it in some
> cases.
>
Is there documentation for this somewhere ? Msi documentation
(MSI-HOWTO.txt) doesn't say anything about a different interrupt
handler etc.
>> Checks is any interrupts are pending are different for MSI. It may be
>> not hard to do (it's probably easier than for real interrupts), but it
>> has to be figured out. Somebody has to do it right. Until MSI is
>> supported by the interrupt handler, it should not be enabled by the
>> driver, or we get a non-functioning driver for AR5006.
>
> Sure, that's fine. I just wanted to make sure that there weren't some weird
> generic MSI problems.
>
I was referring to this post...
http://lkml.org/lkml/2008/6/24/150
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On Wednesday, July 16, 2008 9:15 am Nick Kossifidis wrote:
> It's ok for now, but have in mind that on my upcoming patch series i'm
> disabling msi (commented out) since it results no interrupts on pci-e
> cards (seems there is a bug in kernel's msi implementation).
Hm, would be good to get details here. MSI is being used by other drivers
successfully...
Jesse
2008/7/15 Jiri Slaby <[email protected]>:
> Don't sync
> - coherent mapping (descriptors)
> - before unmap, it's useless
> - (wrongly anyway -- for_cpu) beacon skb, it's just mapped,
> so by the device yet
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Luis R. Rodriguez <[email protected]>
> ---
> drivers/net/wireless/ath5k/base.c | 11 -----------
> 1 files changed, 0 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 4874a6f..713ee99 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -1705,10 +1705,6 @@ ath5k_tasklet_rx(unsigned long data)
> skb = bf->skb;
> ds = bf->desc;
>
> - /* TODO only one segment */
> - pci_dma_sync_single_for_cpu(sc->pdev, sc->desc_daddr,
> - sc->desc_len, PCI_DMA_FROMDEVICE);
> -
> /*
> * last buffer must not be freed to ensure proper hardware
> * function. When the hardware finishes also a packet next to
> @@ -1772,8 +1768,6 @@ ath5k_tasklet_rx(unsigned long data)
> goto next;
> }
> accept:
> - pci_dma_sync_single_for_cpu(sc->pdev, bf->skbaddr,
> - rs.rs_datalen, PCI_DMA_FROMDEVICE);
> pci_unmap_single(sc->pdev, bf->skbaddr, sc->rxbufsize,
> PCI_DMA_FROMDEVICE);
> bf->skb = NULL;
> @@ -1861,9 +1855,6 @@ ath5k_tx_processq(struct ath5k_softc *sc, struct ath5k_txq *txq)
> list_for_each_entry_safe(bf, bf0, &txq->q, list) {
> ds = bf->desc;
>
> - /* TODO only one segment */
> - pci_dma_sync_single_for_cpu(sc->pdev, sc->desc_daddr,
> - sc->desc_len, PCI_DMA_FROMDEVICE);
> ret = sc->ah->ah_proc_tx_desc(sc->ah, ds, &ts);
> if (unlikely(ret == -EINPROGRESS))
> break;
> @@ -2036,8 +2027,6 @@ ath5k_beacon_send(struct ath5k_softc *sc)
> ATH5K_WARN(sc, "beacon queue %u didn't stop?\n", sc->bhalq);
> /* NB: hw still stops DMA, so proceed */
> }
> - pci_dma_sync_single_for_cpu(sc->pdev, bf->skbaddr, bf->skb->len,
> - PCI_DMA_TODEVICE);
>
> ath5k_hw_put_tx_buf(ah, sc->bhalq, bf->daddr);
> ath5k_hw_tx_start(ah, sc->bhalq);
> --
> 1.5.6.2
>
>
Acked-by: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
Make sure that the irq is not in progress after stop. This means
two things:
- ensure the intr setting register is set by flushing posted values
- call synchronize_irq() after that
Also flush stop tx write, inform callers of the tx stop about still
pending transfers (unsuccessful stop) and finally don't wait another
3ms in ath5k_rx_stop, since ath5k_hw_stop_rx_dma ensures transfer to
be finished.
Make sure all writes will be ordered in respect to locks by mmiowb().
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 13 +++++++++++--
drivers/net/wireless/ath5k/hw.c | 4 ++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 4d9ff97..4874a6f 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -43,7 +43,9 @@
#include <linux/version.h>
#include <linux/module.h>
#include <linux/delay.h>
+#include <linux/hardirq.h>
#include <linux/if.h>
+#include <linux/io.h>
#include <linux/netdevice.h>
#include <linux/cache.h>
#include <linux/pci.h>
@@ -1250,6 +1252,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf)
txq->link = &ds->ds_link;
ath5k_hw_tx_start(ah, txq->qnum);
+ mmiowb();
spin_unlock_bh(&txq->lock);
return 0;
@@ -1584,7 +1587,6 @@ ath5k_rx_stop(struct ath5k_softc *sc)
ath5k_hw_stop_pcu_recv(ah); /* disable PCU */
ath5k_hw_set_rx_filter(ah, 0); /* clear recv filter */
ath5k_hw_stop_rx_dma(ah); /* disable DMA engine */
- mdelay(3); /* 3ms is long enough for 1 frame */
ath5k_debug_printrxbuffs(sc, ah);
@@ -2259,6 +2261,7 @@ ath5k_init(struct ath5k_softc *sc)
ret = 0;
done:
+ mmiowb();
mutex_unlock(&sc->lock);
return ret;
}
@@ -2291,6 +2294,7 @@ ath5k_stop_locked(struct ath5k_softc *sc)
if (!test_bit(ATH_STAT_INVALID, sc->status)) {
ath5k_led_off(sc);
ath5k_hw_set_intr(ah, 0);
+ synchronize_irq(sc->pdev->irq);
}
ath5k_txq_cleanup(sc);
if (!test_bit(ATH_STAT_INVALID, sc->status)) {
@@ -2340,6 +2344,7 @@ ath5k_stop_hw(struct ath5k_softc *sc)
}
}
ath5k_txbuf_free(sc, sc->bbuf);
+ mmiowb();
mutex_unlock(&sc->lock);
del_timer_sync(&sc->calib_tim);
@@ -2805,6 +2810,7 @@ ath5k_config_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
/* XXX: assoc id is set to 0 for now, mac80211 doesn't have
* a clean way of letting us retrieve this yet. */
ath5k_hw_set_associd(ah, ah->ah_bssid, 0);
+ mmiowb();
}
mutex_unlock(&sc->lock);
@@ -2981,6 +2987,7 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
}
unlock:
+ mmiowb();
mutex_unlock(&sc->lock);
return ret;
}
@@ -3054,8 +3061,10 @@ ath5k_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb)
ret = ath5k_beacon_setup(sc, sc->bbuf);
if (ret)
sc->bbuf->skb = NULL;
- else
+ else {
ath5k_beacon_config(sc);
+ mmiowb();
+ }
end:
mutex_unlock(&sc->lock);
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index c6d12c5..7ca87a5 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -1440,6 +1440,7 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
/* Stop queue */
ath5k_hw_reg_write(ah, tx_queue, AR5K_CR);
+ ath5k_hw_reg_read(ah, AR5K_CR);
} else {
/*
* Schedule TX disable and wait until queue is empty
@@ -1456,6 +1457,8 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
/* Clear register */
ath5k_hw_reg_write(ah, 0, AR5K_QCU_TXD);
+ if (pending)
+ return -EBUSY;
}
/* TODO: Check for success else return error */
@@ -1716,6 +1719,7 @@ enum ath5k_int ath5k_hw_set_intr(struct ath5k_hw *ah, enum ath5k_int new_mask)
/* ..re-enable interrupts */
ath5k_hw_reg_write(ah, AR5K_IER_ENABLE, AR5K_IER);
+ ath5k_hw_reg_read(ah, AR5K_IER);
return old_mask;
}
--
1.5.6.2
MjAwOC83LzE1IEppcmkgU2xhYnkgPGppcmlzbGFieUBnbWFpbC5jb20+Ogo+IFdoZW4gc2lnbmFs
IGlzIG5vaXN5LCBoYXJkd2FyZSBjYW4gdXNlIGFsbCBSWCBidWZmZXJzIGFuZCBzaW5jZSB0aGUg
bGFzdAo+IGVudHJ5IGluIHRoZSBsaXN0IGlzIHNlbGYtbGlua2VkLCBpdCBvdmVyd3JpdGVzIHRo
ZSBlbnRyeSB1bnRpbCB3ZSBsaW5rCj4gbmV3IGJ1ZmZlcnMuCj4KPiBFbnN1cmUgdGhhdCB3ZSBk
b24ndCBmcmVlIHRoaXMgbGFzdCBvbmUgdW50aWwgd2UgYXJlIDEwMCUgc3VyZSB0aGF0IGl0Cj4g
aXMgbm90IHVzZWQgYnkgdGhlIGhhcmR3YXJlIGFueW1vcmUgdG8gbm90IGNhdXNlIG1lbW9yeSBj
dXJydXB0aW9uIGFzCj4gY2FuIGJlIHNlZW4gYmVsb3cuCj4KPiBUaGlzIGlzIGRvbmUgYnkgY2hl
Y2tpbmcgbmV4dCBidWZmZXIgaW4gdGhlIGxpc3QuIEV2ZW4gYWZ0ZXIgdGhhdCB3ZQo+IGtub3cg
dGhhdCB0aGUgaGFyZHdhcmUgcmVmZXRjaGVkIHRoZSBuZXcgbGluayBhbmQgcHJvY2VlZGVkIGZ1
cnRoZXIKPiAodGhlIG5leHQgYnVmZmVyIGlzIHJlYWR5KSB3ZSBjYW4gZmluYWxseSBmcmVlIHRo
ZSBvdmVyd3JpdHRlbiBidWZmZXIuCj4KPiBXZSBkaXNjYXJkIGl0IHNpbmNlIHRoZSBzdGF0dXMg
aW4gaXRzIGRlc2NyaXB0b3IgaXMgb3ZlcndyaXR0ZW4gKE9SLWVkCj4gYnkgbmV3IHN0YXR1cykg
dG9vLgo+Cj4gPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KPiBCVUcga21hbGxvYy00MDk2OiBQb2lzb24g
b3ZlcndyaXR0ZW4KPiAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQo+Cj4gSU5GTzogMHhmZmZmODEwMDY3
NDE5MDYwLTB4ZmZmZjgxMDA2NzQxOTY2Ny4gRmlyc3QgYnl0ZSAweDggaW5zdGVhZCBvZiAweDZi
Cj4gSU5GTzogQWxsb2NhdGVkIGluIGRldl9hbGxvY19za2IrMHgxOC8weDMwIGFnZT0xMTE4IGNw
dT0xIHBpZD0wCj4gSU5GTzogRnJlZWQgaW4gc2tiX3JlbGVhc2VfZGF0YSsweDg1LzB4ZDAgYWdl
PTExMDUgY3B1PTEgcGlkPTM3MTgKPiBJTkZPOiBTbGFiIDB4ZmZmZmUyMDAwMTlkMDYwMCBvYmpl
Y3RzPTcgdXNlZD0wIGZwPTB4ZmZmZjgxMDA2NzQxOTA0OCBmbGFncz0weDQwMDAwMDAwMDAwMDIw
YzMKPiBJTkZPOiBPYmplY3QgMHhmZmZmODEwMDY3NDE5MDQ4IEBvZmZzZXQ9NDE2OCBmcD0weGZm
ZmY4MTAwNjc0MWMxMjAKPgo+IEJ5dGVzIGI0IDB4ZmZmZjgxMDA2NzQxOTAzODogIDRmIDBiIDAy
IDAwIDAxIDAwIDAwIDAwIDVhIDVhIDVhIDVhIDVhIDVhIDVhIDVhIE8uLi4uLi4uWlpaWlpaWloK
PiAgT2JqZWN0IDB4ZmZmZjgxMDA2NzQxOTA0ODogIDZiIDZiIDZiIDZiIDZiIDZiIDZiIDZiIDZi
IDZiIDZiIDZiIDZiIDZiIDZiIDZiIGtra2tra2tra2tra2tra2sKPiAgT2JqZWN0IDB4ZmZmZjgx
MDA2NzQxOTA1ODogIDZiIDZiIDZiIDZiIDZiIDZiIDZiIDZiIDA4IDQyIDMwIDAwIDAwIDBiIDZi
IDgwIGtra2tra2trLkIwLi4uay4KPiAgT2JqZWN0IDB4ZmZmZjgxMDA2NzQxOTA2ODogIGYwIDVk
IDAwIDRmIDYyIDA4IGEzIDY0IDAwIDBjIDQyIDE2IDUyIGU0IGYwIDVhIDM2MF0uT2IuMjQzZC4u
Qi5SMzQ0MzYwWgo+ICBPYmplY3QgMHhmZmZmODEwMDY3NDE5MDc4OiAgNjggODEgMDAgMDAgN2Ig
YTUgYjQgYmUgN2QgM2IgOGYgNTMgY2QgZDUgZGUgMTIgaC4uLnsyNDUyNjQyNzZ9Oy5TMzE1MzI1
MzM2Lgo+ICBPYmplY3QgMHhmZmZmODEwMDY3NDE5MDg4OiAgOTYgMTAgMGIgODkgNDggNTQgMjMg
NDEgMGYgNGUgMmQgYjkgMzcgYzMgY2IgMjkgLi4uLkhUI0EuTi0yNzE3MzAzMzEzKQo+ICBPYmpl
Y3QgMHhmZmZmODEwMDY3NDE5MDk4OiAgZDEgZTAgZGUgMTQgOGEgNTcgMmEgY2MgM2IgNDQgMGQg
NzggN2EgMTkgMTIgMTUgMzIxMzQwMzM2Li5XKjMxNDtELnh6Li4uCj4gIE9iamVjdCAweGZmZmY4
MTAwNjc0MTkwYTg6ICBhOSBlYyBkNCAzNSBhOCAxMCBlYyA4YyA0MCBhNyAwNiAwYSA1MSBhNyA0
OCBiYiAyNTEzNTQzMjQ1MjUwLjM1NC5AMjQ3Li5RMjQ3SDI3Mwo+ICBPYmplY3QgMHhmZmZmODEw
MDY3NDE5MGI4OiAgM2UgY2YgYTEgYzcgMzggNjAgNjMgM2YgNTEgMTUgYzcgMjAgZWIgYmEgNjUg
MzAgPs+hMzA3OGBjP1EuMzA3LjM1MzI3MmUwCj4gIFJlZHpvbmUgMHhmZmZmODEwMDY3NDFhMDQ4
OiAgYmIgYmIgYmIgYmIgYmIgYmIgYmIgYmIgICAgICAgICAgICAgICAgICAgICAgICAgMjczMjcz
MjczMjczMjczMjczMjczMjczCj4gIFBhZGRpbmcgMHhmZmZmODEwMDY3NDFhMDg4OiAgNWEgNWEg
NWEgNWEgNWEgNWEgNWEgNWEgICAgICAgICAgICAgICAgICAgICAgICAgWlpaWlpaWloKPiBQaWQ6
IDMyOTcsIGNvbW06IGF0aDVrX3BjaSBOb3QgdGFpbnRlZCAyLjYuMjYtcmM4LW1tMV82NCAjNDI3
Cj4KPiBDYWxsIFRyYWNlOgo+ICBbPGZmZmZmZmZmODAyYTczMDY+XSBwcmludF90cmFpbGVyKzB4
ZjYvMHgxNTAKPiAgWzxmZmZmZmZmZjgwMmE3NDg1Pl0gY2hlY2tfYnl0ZXNfYW5kX3JlcG9ydCsw
eDEyNS8weDE4MAo+ICBbPGZmZmZmZmZmODAyYTc1ZGM+XSBjaGVja19vYmplY3QrMHhhYy8weDI2
MAo+ICBbPGZmZmZmZmZmODAyYTkzMDg+XSBfX3NsYWJfYWxsb2MrMHgzNjgvMHg2ZDAKPiAgWzxm
ZmZmZmZmZjgwNTQ0ZjgyPl0gPyB3aXJlbGVzc19zZW5kX2V2ZW50KzB4MTQyLzB4MzEwCj4gIFs8
ZmZmZmZmZmY4MDRiMWJkND5dID8gX19hbGxvY19za2IrMHg0NC8weDE1MAo+ICBbPGZmZmZmZmZm
ODA1NDRmODI+XSA/IHdpcmVsZXNzX3NlbmRfZXZlbnQrMHgxNDIvMHgzMTAKPiAgWzxmZmZmZmZm
ZjgwMmFhODUzPl0gX19rbWFsbG9jX3RyYWNrX2NhbGxlcisweGMzLzB4ZjAKPiAgWzxmZmZmZmZm
ZjgwNGIxYmZlPl0gX19hbGxvY19za2IrMHg2ZS8weDE1MAo+IFsuLi4gc3RhY2sgc25pcHBlZF0K
Pgo+IEZJWCBrbWFsbG9jLTQwOTY6IFJlc3RvcmluZyAweGZmZmY4MTAwNjc0MTkwNjAtMHhmZmZm
ODEwMDY3NDE5NjY3PTB4NmIKPgo+IEZJWCBrbWFsbG9jLTQwOTY6IE1hcmtpbmcgYWxsIG9iamVj
dHMgdXNlZAo+Cj4gU2lnbmVkLW9mZi1ieTogSmlyaSBTbGFieSA8amlyaXNsYWJ5QGdtYWlsLmNv
bT4KPiBDYzogTmljayBLb3NzaWZpZGlzIDxtaWNrZmxlbW1AZ21haWwuY29tPgo+IENjOiBMdWlz
IFIuIFJvZHJpZ3VleiA8bWNncm9mQGdtYWlsLmNvbT4KPiAtLS0KPiAgZHJpdmVycy9uZXQvd2ly
ZWxlc3MvYXRoNWsvYmFzZS5jIHwgICAzMiArKysrKysrKysrKysrKysrKysrKysrKysrLS0tLS0t
LQo+ICBkcml2ZXJzL25ldC93aXJlbGVzcy9hdGg1ay9iYXNlLmggfCAgICAyICstCj4gIDIgZmls
ZXMgY2hhbmdlZCwgMjYgaW5zZXJ0aW9ucygrKSwgOCBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1n
aXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGg1ay9iYXNlLmMgYi9kcml2ZXJzL25ldC93aXJl
bGVzcy9hdGg1ay9iYXNlLmMKPiBpbmRleCAxMmE5NDQzLi5lOWVjMjg0IDEwMDY0NAo+IC0tLSBh
L2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aDVrL2Jhc2UuYwo+ICsrKyBiL2RyaXZlcnMvbmV0L3dp
cmVsZXNzL2F0aDVrL2Jhc2UuYwo+IEBAIC0xNjgzLDIwICsxNjgzLDIxIEBAIGF0aDVrX3Rhc2ts
ZXRfcngodW5zaWduZWQgbG9uZyBkYXRhKQo+ICAgICAgICBzdHJ1Y3QgYXRoNWtfcnhfc3RhdHVz
IHJzID0ge307Cj4gICAgICAgIHN0cnVjdCBza19idWZmICpza2I7Cj4gICAgICAgIHN0cnVjdCBh
dGg1a19zb2Z0YyAqc2MgPSAodm9pZCAqKWRhdGE7Cj4gLSAgICAgICBzdHJ1Y3QgYXRoNWtfYnVm
ICpiZjsKPiArICAgICAgIHN0cnVjdCBhdGg1a19idWYgKmJmLCAqYmZfbGFzdDsKPiAgICAgICAg
c3RydWN0IGF0aDVrX2Rlc2MgKmRzOwo+ICAgICAgICBpbnQgcmV0Owo+ICAgICAgICBpbnQgaGRy
bGVuOwo+ICAgICAgICBpbnQgcGFkOwo+Cj4gICAgICAgIHNwaW5fbG9jaygmc2MtPnJ4YnVmbG9j
ayk7Cj4gKyAgICAgICBpZiAobGlzdF9lbXB0eSgmc2MtPnJ4YnVmKSkgewo+ICsgICAgICAgICAg
ICAgICBBVEg1S19XQVJOKHNjLCAiZW1wdHkgcnggYnVmIHBvb2xcbiIpOwo+ICsgICAgICAgICAg
ICAgICBnb3RvIHVubG9jazsKPiArICAgICAgIH0KPiArICAgICAgIGJmX2xhc3QgPSBsaXN0X2Vu
dHJ5KHNjLT5yeGJ1Zi5wcmV2LCBzdHJ1Y3QgYXRoNWtfYnVmLCBsaXN0KTsKPiAgICAgICAgZG8g
ewo+ICAgICAgICAgICAgICAgIHJ4cy5mbGFnID0gMDsKPgo+IC0gICAgICAgICAgICAgICBpZiAo
dW5saWtlbHkobGlzdF9lbXB0eSgmc2MtPnJ4YnVmKSkpIHsKPiAtICAgICAgICAgICAgICAgICAg
ICAgICBBVEg1S19XQVJOKHNjLCAiZW1wdHkgcnggYnVmIHBvb2xcbiIpOwo+IC0gICAgICAgICAg
ICAgICAgICAgICAgIGJyZWFrOwo+IC0gICAgICAgICAgICAgICB9Cj4gICAgICAgICAgICAgICAg
YmYgPSBsaXN0X2ZpcnN0X2VudHJ5KCZzYy0+cnhidWYsIHN0cnVjdCBhdGg1a19idWYsIGxpc3Qp
Owo+ICAgICAgICAgICAgICAgIEJVR19PTihiZi0+c2tiID09IE5VTEwpOwo+ICAgICAgICAgICAg
ICAgIHNrYiA9IGJmLT5za2I7Cj4gQEAgLTE3MDYsOCArMTcwNywyNCBAQCBhdGg1a190YXNrbGV0
X3J4KHVuc2lnbmVkIGxvbmcgZGF0YSkKPiAgICAgICAgICAgICAgICBwY2lfZG1hX3N5bmNfc2lu
Z2xlX2Zvcl9jcHUoc2MtPnBkZXYsIHNjLT5kZXNjX2RhZGRyLAo+ICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBzYy0+ZGVzY19sZW4sIFBDSV9ETUFfRlJPTURFVklDRSk7Cj4KPiAtICAg
ICAgICAgICAgICAgaWYgKHVubGlrZWx5KGRzLT5kc19saW5rID09IGJmLT5kYWRkcikpIC8qIHRo
aXMgaXMgdGhlIGVuZCAqLwo+IC0gICAgICAgICAgICAgICAgICAgICAgIGJyZWFrOwo+ICsgICAg
ICAgICAgICAgICAvKgo+ICsgICAgICAgICAgICAgICAgKiBsYXN0IGJ1ZmZlciBtdXN0IG5vdCBi
ZSBmcmVlZCB0byBlbnN1cmUgcHJvcGVyIGhhcmR3YXJlCj4gKyAgICAgICAgICAgICAgICAqIGZ1
bmN0aW9uLiBXaGVuIHRoZSBoYXJkd2FyZSBmaW5pc2hlcyBhbHNvIGEgcGFja2V0IG5leHQgdG8K
PiArICAgICAgICAgICAgICAgICogaXQsIHdlIGFyZSBzdXJlLCBpdCBkb2Vzbid0IHVzZSBpdCBh
bnltb3JlIGFuZCB3ZSBjYW4gZ28gb24uCj4gKyAgICAgICAgICAgICAgICAqLwo+ICsgICAgICAg
ICAgICAgICBpZiAoYmZfbGFzdCA9PSBiZikKPiArICAgICAgICAgICAgICAgICAgICAgICBiZi0+
ZmxhZ3MgfD0gMTsKPiArICAgICAgICAgICAgICAgaWYgKGJmLT5mbGFncykgewo+ICsgICAgICAg
ICAgICAgICAgICAgICAgIHN0cnVjdCBhdGg1a19idWYgKmJmX25leHQgPSBsaXN0X2VudHJ5KGJm
LT5saXN0Lm5leHQsCj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHN0
cnVjdCBhdGg1a19idWYsIGxpc3QpOwo+ICsgICAgICAgICAgICAgICAgICAgICAgIHJldCA9IHNj
LT5haC0+YWhfcHJvY19yeF9kZXNjKHNjLT5haCwgYmZfbmV4dC0+ZGVzYywKPiArICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJnJzKTsKPiArICAgICAgICAgICAgICAgICAg
ICAgICBpZiAocmV0KQo+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgYnJlYWs7Cj4g
KyAgICAgICAgICAgICAgICAgICAgICAgYmYtPmZsYWdzICY9IH4xOwo+ICsgICAgICAgICAgICAg
ICAgICAgICAgIC8qIHNraXAgdGhlIG92ZXJ3cml0dGVuIG9uZSAoZXZlbiBzdGF0dXMgaXMgbWFy
dGlhbikgKi8KPiArICAgICAgICAgICAgICAgICAgICAgICBnb3RvIG5leHQ7Cj4gKyAgICAgICAg
ICAgICAgIH0KPgo+ICAgICAgICAgICAgICAgIHJldCA9IHNjLT5haC0+YWhfcHJvY19yeF9kZXNj
KHNjLT5haCwgZHMsICZycyk7Cj4gICAgICAgICAgICAgICAgaWYgKHVubGlrZWx5KHJldCA9PSAt
RUlOUFJPR1JFU1MpKQo+IEBAIC0xODE3LDYgKzE4MzQsNyBAQCBhY2NlcHQ6Cj4gIG5leHQ6Cj4g
ICAgICAgICAgICAgICAgbGlzdF9tb3ZlX3RhaWwoJmJmLT5saXN0LCAmc2MtPnJ4YnVmKTsKPiAg
ICAgICAgfSB3aGlsZSAoYXRoNWtfcnhidWZfc2V0dXAoc2MsIGJmKSA9PSAwKTsKPiArdW5sb2Nr
Ogo+ICAgICAgICBzcGluX3VubG9jaygmc2MtPnJ4YnVmbG9jayk7Cj4gIH0KPgo+IGRpZmYgLS1n
aXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGg1ay9iYXNlLmggYi9kcml2ZXJzL25ldC93aXJl
bGVzcy9hdGg1ay9iYXNlLmgKPiBpbmRleCA0N2Y0MTRiLi5kN2UwM2U2IDEwMDY0NAo+IC0tLSBh
L2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aDVrL2Jhc2UuaAo+ICsrKyBiL2RyaXZlcnMvbmV0L3dp
cmVsZXNzL2F0aDVrL2Jhc2UuaAo+IEBAIC01Niw3ICs1Niw3IEBACj4KPiAgc3RydWN0IGF0aDVr
X2J1ZiB7Cj4gICAgICAgIHN0cnVjdCBsaXN0X2hlYWQgICAgICAgIGxpc3Q7Cj4gLSAgICAgICB1
bnNpZ25lZCBpbnQgICAgICAgICAgICBmbGFnczsgIC8qIHR4IGRlc2NyaXB0b3IgZmxhZ3MgKi8K
PiArICAgICAgIHVuc2lnbmVkIGludCAgICAgICAgICAgIGZsYWdzOyAgLyogcnggZGVzY3JpcHRv
ciBmbGFncyAqLwo+ICAgICAgICBzdHJ1Y3QgYXRoNWtfZGVzYyAgICAgICAqZGVzYzsgIC8qIHZp
cnR1YWwgYWRkciBvZiBkZXNjICovCj4gICAgICAgIGRtYV9hZGRyX3QgICAgICAgICAgICAgIGRh
ZGRyOyAgLyogcGh5c2ljYWwgYWRkciBvZiBkZXNjICovCj4gICAgICAgIHN0cnVjdCBza19idWZm
ICAgICAgICAgICpza2I7ICAgLyogc2tidWZmIGZvciBidWYgKi8KPiAtLQo+IDEuNS42LjIKPgo+
CgpOaWNlIGNhdGNoIDstKQoKQWNrZWQtYnk6IE5pY2sgS29zc2lmaWRpcyA8bWlja2ZsZW1tQGdt
YWlsLmNvbT4KCi0tIApHUEcgSUQ6IDB4RDIxREIyREIKQXMgeW91IHJlYWQgdGhpcyBwb3N0IGds
b2JhbCBlbnRyb3B5IHJpc2VzLiBIYXZlIEZ1biA7LSkKTmljawo=
2008/7/15 Jiri Slaby <[email protected]>:
> - free and re-request irq since it might have changed during suspend
> - disable and enable msi
> - don't set D0 state of the device, it's already done by the PCI layer
> - do restore_state before enable_device, it's safer
> - check ath5k_init return value
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Luis R. Rodriguez <[email protected]>
> Cc: Jesse Barnes <[email protected]>
> ---
> drivers/net/wireless/ath5k/base.c | 26 +++++++++++++++++++++-----
> 1 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 713ee99..d58a883 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -593,6 +593,9 @@ ath5k_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> ath5k_led_off(sc);
>
> ath5k_stop_hw(sc);
> +
> + free_irq(pdev->irq, sc);
> + pci_disable_msi(pdev);
> pci_save_state(pdev);
> pci_disable_device(pdev);
> pci_set_power_state(pdev, PCI_D3hot);
> @@ -608,15 +611,12 @@ ath5k_pci_resume(struct pci_dev *pdev)
> struct ath5k_hw *ah = sc->ah;
> int i, err;
>
> - err = pci_set_power_state(pdev, PCI_D0);
> - if (err)
> - return err;
> + pci_restore_state(pdev);
>
> err = pci_enable_device(pdev);
> if (err)
> return err;
>
> - pci_restore_state(pdev);
> /*
> * Suspend/Resume resets the PCI configuration space, so we have to
> * re-disable the RETRY_TIMEOUT register (0x41) to keep
> @@ -624,7 +624,17 @@ ath5k_pci_resume(struct pci_dev *pdev)
> */
> pci_write_config_byte(pdev, 0x41, 0);
>
> - ath5k_init(sc);
> + pci_enable_msi(pdev);
> +
> + err = request_irq(pdev->irq, ath5k_intr, IRQF_SHARED, "ath", sc);
> + if (err) {
> + ATH5K_ERR(sc, "request_irq failed\n");
> + goto err_msi;
> + }
> +
> + err = ath5k_init(sc);
> + if (err)
> + goto err_irq;
> ath5k_led_enable(sc);
>
> /*
> @@ -638,6 +648,12 @@ ath5k_pci_resume(struct pci_dev *pdev)
> ath5k_hw_reset_key(ah, i);
>
> return 0;
> +err_irq:
> + free_irq(pdev->irq, sc);
> +err_msi:
> + pci_disable_msi(pdev);
> + pci_disable_device(pdev);
> + return err;
> }
> #endif /* CONFIG_PM */
>
> --
> 1.5.6.2
>
>
It's ok for now, but have in mind that on my upcoming patch series i'm
disabling msi (commented out) since it results no interrupts on pci-e
cards (seems there is a bug in kernel's msi implementation).
Acked-by: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick