2008-07-18 16:28:57

by Pavel Roskin

[permalink] [raw]
Subject: [PATCH] ath5k: don't enable MSI, we cannot handle it yet

This fixes support for AR5006 chipset, which supports MSI
---

drivers/net/wireless/ath5k/base.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 635b9ac..e57905c 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -487,9 +487,6 @@ ath5k_pci_probe(struct pci_dev *pdev,
/* Set private data */
pci_set_drvdata(pdev, hw);

- /* Enable msi for devices that support it */
- pci_enable_msi(pdev);
-
/* Setup interrupt handler */
ret = request_irq(pdev->irq, ath5k_intr, IRQF_SHARED, "ath", sc);
if (ret) {
@@ -567,7 +564,6 @@ err_ah:
err_irq:
free_irq(pdev->irq, sc);
err_free:
- pci_disable_msi(pdev);
ieee80211_free_hw(hw);
err_map:
pci_iounmap(pdev, mem);
@@ -589,7 +585,6 @@ ath5k_pci_remove(struct pci_dev *pdev)
ath5k_detach(pdev, hw);
ath5k_hw_detach(sc->ah);
free_irq(pdev->irq, sc);
- pci_disable_msi(pdev);
pci_iounmap(pdev, sc->iobase);
pci_release_region(pdev, 0);
pci_disable_device(pdev);


2008-07-18 23:49:44

by Kyle McMartin

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: don't enable MSI, we cannot handle it yet

On Fri, Jul 18, 2008 at 08:57:39PM +0300, Nick Kossifidis wrote:
> > MSI is a nice thing, but we cannot enable it without changing the
> > interrupt handler. If we do it, we break MSI capable hardware,
> > specifically AR5006 chipset.
> >
> > Signed-off-by: Pavel Roskin <[email protected]>
>
> This is needed for all pci-e devices since enabling msi results no interrupts...
>
> Acked-by: Nick Kossifidis <[email protected]>
>

This is likely caused by a bug being discussed on linux-pci right now.
Basically the way we're masking MSI can result in INTn interrupts
getting turned back on, with no handler attached.

http://marc.info/?t=121430463700001&r=1&w=2

regards, Kyle

2008-07-18 17:57:40

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: don't enable MSI, we cannot handle it yet

2008/7/18 Pavel Roskin <[email protected]>:
> MSI is a nice thing, but we cannot enable it without changing the
> interrupt handler. If we do it, we break MSI capable hardware,
> specifically AR5006 chipset.
>
> Signed-off-by: Pavel Roskin <[email protected]>
> ---
>
> drivers/net/wireless/ath5k/base.c | 5 -----
> 1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 635b9ac..e57905c 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -487,9 +487,6 @@ ath5k_pci_probe(struct pci_dev *pdev,
> /* Set private data */
> pci_set_drvdata(pdev, hw);
>
> - /* Enable msi for devices that support it */
> - pci_enable_msi(pdev);
> -
> /* Setup interrupt handler */
> ret = request_irq(pdev->irq, ath5k_intr, IRQF_SHARED, "ath", sc);
> if (ret) {
> @@ -567,7 +564,6 @@ err_ah:
> err_irq:
> free_irq(pdev->irq, sc);
> err_free:
> - pci_disable_msi(pdev);
> ieee80211_free_hw(hw);
> err_map:
> pci_iounmap(pdev, mem);
> @@ -589,7 +585,6 @@ ath5k_pci_remove(struct pci_dev *pdev)
> ath5k_detach(pdev, hw);
> ath5k_hw_detach(sc->ah);
> free_irq(pdev->irq, sc);
> - pci_disable_msi(pdev);
> pci_iounmap(pdev, sc->iobase);
> pci_release_region(pdev, 0);
> pci_disable_device(pdev);
>
>
> --
> Regards,
> Pavel Roskin
>

This is needed for all pci-e devices since enabling msi results no interrupts...

Acked-by: Nick Kossifidis <[email protected]>

--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2008-07-18 16:50:00

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] ath5k: don't enable MSI, we cannot handle it yet

On Friday 18 July 2008 18:44:47 Pavel Roskin wrote:
> On Fri, 2008-07-18 at 12:28 -0400, Pavel Roskin wrote:
> > This fixes support for AR5006 chipset, which supports MSI
>
> Sorry, I found Documentation/stable_kernel_rules.txt minutes after
> sending the patch. We need to get it to the Linus' tree first, then to
> the stable tree. I hope it won't take long.
>
> To preempt possible questions: MSI is a nice thing, but we cannot enable
> it without changing the interrupt handler. If we do it, we break MSI
> capable hardware.

Please include this into the patch changelog. It's highly confusing otherwise ;)

--
Greetings Michael.

2008-07-18 16:44:49

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH] ath5k: don't enable MSI, we cannot handle it yet

On Fri, 2008-07-18 at 12:28 -0400, Pavel Roskin wrote:
> This fixes support for AR5006 chipset, which supports MSI

Sorry, I found Documentation/stable_kernel_rules.txt minutes after
sending the patch. We need to get it to the Linus' tree first, then to
the stable tree. I hope it won't take long.

To preempt possible questions: MSI is a nice thing, but we cannot enable
it without changing the interrupt handler. If we do it, we break MSI
capable hardware.

--
Regards,
Pavel Roskin

Subject: Re: [PATCH v2] ath5k: don't enable MSI, we cannot handle it yet

On Fri, 18 Jul 2008, Nick Kossifidis wrote:
> 2008/7/18 Pavel Roskin <[email protected]>:
> > MSI is a nice thing, but we cannot enable it without changing the
> > interrupt handler. If we do it, we break MSI capable hardware,
> > specifically AR5006 chipset.
> >
> > Signed-off-by: Pavel Roskin <[email protected]>
[...]
> Acked-by: Nick Kossifidis <[email protected]>
Cc: [email protected]

So that they will pick it up automatically when it hits mainline, and it
also serves as an in-band documentation that it is something that needs to
go/went to -stable.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2008-07-18 16:57:01

by Pavel Roskin

[permalink] [raw]
Subject: [PATCH v2] ath5k: don't enable MSI, we cannot handle it yet

MSI is a nice thing, but we cannot enable it without changing the
interrupt handler. If we do it, we break MSI capable hardware,
specifically AR5006 chipset.

Signed-off-by: Pavel Roskin <[email protected]>
---

drivers/net/wireless/ath5k/base.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 635b9ac..e57905c 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -487,9 +487,6 @@ ath5k_pci_probe(struct pci_dev *pdev,
/* Set private data */
pci_set_drvdata(pdev, hw);

- /* Enable msi for devices that support it */
- pci_enable_msi(pdev);
-
/* Setup interrupt handler */
ret = request_irq(pdev->irq, ath5k_intr, IRQF_SHARED, "ath", sc);
if (ret) {
@@ -567,7 +564,6 @@ err_ah:
err_irq:
free_irq(pdev->irq, sc);
err_free:
- pci_disable_msi(pdev);
ieee80211_free_hw(hw);
err_map:
pci_iounmap(pdev, mem);
@@ -589,7 +585,6 @@ ath5k_pci_remove(struct pci_dev *pdev)
ath5k_detach(pdev, hw);
ath5k_hw_detach(sc->ah);
free_irq(pdev->irq, sc);
- pci_disable_msi(pdev);
pci_iounmap(pdev, sc->iobase);
pci_release_region(pdev, 0);
pci_disable_device(pdev);


--
Regards,
Pavel Roskin

2008-07-18 21:07:34

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] ath5k: don't enable MSI, we cannot handle it yet

On Fri, Jul 18, 2008 at 06:49:24PM +0200, Michael Buesch wrote:
> On Friday 18 July 2008 18:44:47 Pavel Roskin wrote:
> > On Fri, 2008-07-18 at 12:28 -0400, Pavel Roskin wrote:
> > > This fixes support for AR5006 chipset, which supports MSI
> >
> > Sorry, I found Documentation/stable_kernel_rules.txt minutes after
> > sending the patch. We need to get it to the Linus' tree first, then to
> > the stable tree. I hope it won't take long.
> >
> > To preempt possible questions: MSI is a nice thing, but we cannot enable
> > it without changing the interrupt handler. If we do it, we break MSI
> > capable hardware.
>
> Please include this into the patch changelog. It's highly confusing otherwise ;)

I agree.

Also, to get stuff into -stable easily, just add:
Cc: stable <[email protected]>
to the signed-off-by area in the patch and then when it goes into
Linus's tree, it will automatically be sent to us for inclusion.

Much easier on you and everyone else that way.

thanks,

greg k-h