| Message ID | 1252457551-4909-1-git-send-email-cascardo@holoscopio.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Sep 08, 2009 at 09:52:31PM -0300, Thadeu Lima de Souza Cascardo wrote: > ath5k will try to request irq when resuming and fails if the device > (like a PCMCIA card) has been removed. That's not true, ath5k no longer requests an irq when resuming. > This solves this issue defining a new flag for the status bitmap to > indicate when irq has been successfully requested and does not try to > release it if not. I'd rather not fix it with a status bit. What kernel is this against?
On Tue, Sep 08, 2009 at 11:32:37PM -0400, Bob Copeland wrote: > On Tue, Sep 08, 2009 at 09:52:31PM -0300, Thadeu Lima de Souza Cascardo wrote: > > ath5k will try to request irq when resuming and fails if the device > > (like a PCMCIA card) has been removed. > > That's not true, ath5k no longer requests an irq when resuming. > I've just saw there's a commit by you in the next tree that just removes the irq requesting and releasing in resume/suspend functions. > > This solves this issue defining a new flag for the status bitmap to > > indicate when irq has been successfully requested and does not try to > > release it if not. > > I'd rather not fix it with a status bit. What kernel is this against? > This is against v2.6.31-rc9, so I get a warning with a version that's about to get stable. Sorry I am late in the release cycle. I've used a status bit because the drivers I took a look at did release/request irq in suspend/resume. I couldn't find a message about not doing it was the right thing which I thought I saw in the latest updates to v2.6.31-rcX. I guess it was something just like your commit which I did see some weeks ago. Since this is warning, is this worth backporting to rc? > -- > Bob Copeland %% www.bobcopeland.com > Regards, Cascardo.
On Wed, Sep 09, 2009 at 02:50:06PM -0300, Thadeu Lima de Souza Cascardo wrote: > This is against v2.6.31-rc9, so I get a warning with a version that's > about to get stable. Sorry I am late in the release cycle. Ok yes, that makes sense then. > Since this is warning, is this worth backporting to rc? It's too late probably, but we can send it to stable for 2.6.31.1. Thank you for the patch by the way, just keep in mind that wireless patches should be against the wireless-testing kernel in the future.
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index 029c1bc..c5e2d5b 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -553,6 +553,7 @@ ath5k_pci_probe(struct pci_dev *pdev, ATH5K_ERR(sc, "request_irq failed\n"); goto err_free; } + __set_bit(ATH_STAT_IRQREQUESTED, sc->status); /* Initialize device */ sc->ah = ath5k_hw_attach(sc, id->driver_data); @@ -628,6 +629,7 @@ ath5k_pci_probe(struct pci_dev *pdev, err_ah: ath5k_hw_detach(sc->ah); err_irq: + __clear_bit(ATH_STAT_IRQREQUESTED, sc->status); free_irq(pdev->irq, sc); err_free: ieee80211_free_hw(hw); @@ -650,7 +652,10 @@ ath5k_pci_remove(struct pci_dev *pdev) ath5k_debug_finish_device(sc); ath5k_detach(pdev, hw); ath5k_hw_detach(sc->ah); - free_irq(pdev->irq, sc); + if (test_bit(ATH_STAT_IRQREQUESTED, sc->status)) { + __clear_bit(ATH_STAT_IRQREQUESTED, sc->status); + free_irq(pdev->irq, sc); + } pci_iounmap(pdev, sc->iobase); pci_release_region(pdev, 0); pci_disable_device(pdev); @@ -666,7 +671,10 @@ ath5k_pci_suspend(struct pci_dev *pdev, pm_message_t state) ath5k_led_off(sc); - free_irq(pdev->irq, sc); + if (test_bit(ATH_STAT_IRQREQUESTED, sc->status)) { + __clear_bit(ATH_STAT_IRQREQUESTED, sc->status); + free_irq(pdev->irq, sc); + } pci_save_state(pdev); pci_disable_device(pdev); pci_set_power_state(pdev, PCI_D3hot); @@ -699,6 +707,7 @@ ath5k_pci_resume(struct pci_dev *pdev) ATH5K_ERR(sc, "request_irq failed\n"); goto err_no_irq; } + __set_bit(ATH_STAT_IRQREQUESTED, sc->status); ath5k_led_enable(sc); return 0; diff --git a/drivers/net/wireless/ath/ath5k/base.h b/drivers/net/wireless/ath/ath5k/base.h index f9b7f2f..4a71437 100644 --- a/drivers/net/wireless/ath/ath5k/base.h +++ b/drivers/net/wireless/ath/ath5k/base.h @@ -137,12 +137,13 @@ struct ath5k_softc { size_t desc_len; /* size of TX/RX descriptors */ u16 cachelsz; /* cache line size */ - DECLARE_BITMAP(status, 5); + DECLARE_BITMAP(status, 6); #define ATH_STAT_INVALID 0 /* disable hardware accesses */ #define ATH_STAT_MRRETRY 1 /* multi-rate retry support */ #define ATH_STAT_PROMISC 2 #define ATH_STAT_LEDSOFT 3 /* enable LED gpio status */ #define ATH_STAT_STARTED 4 /* opened & irqs enabled */ +#define ATH_STAT_IRQREQUESTED 5 /* irq requested */ unsigned int filter_flags; /* HW flags, AR5K_RX_FILTER_* */ unsigned int curmode; /* current phy mode */
ath5k will try to request irq when resuming and fails if the device (like a PCMCIA card) has been removed. The driver remove function will, then, be called, trying to free the failed requested irq, resulting in a warning. This solves this issue defining a new flag for the status bitmap to indicate when irq has been successfully requested and does not try to release it if not. Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> --- drivers/net/wireless/ath/ath5k/base.c | 13 +++++++++++-- drivers/net/wireless/ath/ath5k/base.h | 3 ++- 2 files changed, 13 insertions(+), 3 deletions(-)