* RE: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-13 21:45 [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled Bjorn Helgaas
@ 2018-03-13 22:41 ` Luck, Tony
2018-03-14 8:35 ` Christoph Hellwig
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Luck, Tony @ 2018-03-13 22:41 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci@vger.kernel.org
Cc: Mikael Starvik, Jesper Nilsson, Yu, Fenghua, David Howells,
Wysocki, Rafael J, linux-cris-kernel@axis.com,
linux-kernel@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-am33-list@redhat.com
PiBDaGFuZ2UgdGhlIHJlbWFpbmluZyBhcmNoZXMgKGNyaXMsIGZydiwgYW5kIGlhNjQpIHRvIHNr
aXAgSU5UeCBzZXR1cCB3aGVuDQo+IGVpdGhlciBNU0kgb3IgTVNJLVggaXMgZW5hYmxlZC4NCg0K
TXkgaWE2NCBzdGlsbCBib290cyB3aXRoIHRoaXMgcGF0Y2ggYXBwbGllZCAoYW5kIGRvZXNuJ3Qg
aGF2ZSBhbnkgbmV3L2RpZmZlcmVudCBtZXNzYWdlcw0KaW4gdGhlIGNvbnNvbGUgbG9nKS4gIFNv
Og0KDQpUZXN0ZWQtYnk6IFRvbnkgTHVjayA8dG9ueS5sdWNrQGludGVsLmNvbT4NCg0KLVRvbnkN
Cg=
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-13 21:45 [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled Bjorn Helgaas
2018-03-13 22:41 ` Luck, Tony
@ 2018-03-14 8:35 ` Christoph Hellwig
2018-03-14 17:12 ` Bjorn Helgaas
2018-03-14 10:27 ` Rafael J. Wysocki
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2018-03-14 8:35 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, Rafael J. Wysocki, linux-cris-kernel, linux-kernel,
linux-ia64, linux-am33-list
Should this logic go into a little helper so that everyone is kept
in sync?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-14 8:35 ` Christoph Hellwig
@ 2018-03-14 17:12 ` Bjorn Helgaas
0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2018-03-14 17:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-pci, Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, Rafael J. Wysocki, linux-cris-kernel, linux-kernel,
linux-ia64, linux-am33-list
On Wed, Mar 14, 2018 at 01:35:19AM -0700, Christoph Hellwig wrote:
> Should this logic go into a little helper so that everyone is kept
> in sync?
Great point, thanks! What I'd really like is to completely get rid of
most of the pcibios_enable_device() implementations. Most of them
contain nothing that's arch-specific.
I'll look at that some more, but will probably do that as additional
steps on top of this one. That way this patch stays mostly trivial
and obvious as a generalization of 16cf0ebc35dd ("x86/PCI: Do not use
interrupt links for devices using MSI-X") to other affected arches.
Bjorn
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-13 21:45 [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled Bjorn Helgaas
2018-03-13 22:41 ` Luck, Tony
2018-03-14 8:35 ` Christoph Hellwig
@ 2018-03-14 10:27 ` Rafael J. Wysocki
2018-03-14 16:31 ` Andy Shevchenko
2018-03-19 18:57 ` Bjorn Helgaas
4 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2018-03-14 10:27 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci
Cc: Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, linux-cris-kernel, linux-kernel, linux-ia64,
linux-am33-list
On 3/13/2018 10:45 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> If a device already has MSI or MSI-X enabled, there's no need to set up its
> legacy INTx interrupt.
>
> bba6f6fc68e7 ("[PATCH] MSI-X: fix resume crash") changed the cris, frv,
> x86, and ia64 arches to skip INTx setup when MSI is enabled.
>
> 16cf0ebc35dd ("x86/PCI: Do not use interrupt links for devices using
> MSI-X") extended that by changing x86 to also skip INTx setup when MSI-X is
> enabled.
>
> Change the remaining arches (cris, frv, and ia64) to skip INTx setup when
> either MSI or MSI-X is enabled.
>
> Also update mn10300 (which didn't exist at the time of bba6f6fc68e7) to
> follow the same pattern.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> arch/cris/arch-v32/drivers/pci/bios.c | 2 +-
> arch/frv/mb93090-mb00/pci-vdk.c | 2 +-
> arch/ia64/pci/pci.c | 4 ++--
> arch/mn10300/unit-asb2305/pci.c | 8 +++++---
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
> index 6b9e6cfaa29e..c2bed0cc060b 100644
> --- a/arch/cris/arch-v32/drivers/pci/bios.c
> +++ b/arch/cris/arch-v32/drivers/pci/bios.c
> @@ -68,7 +68,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> if ((err = pcibios_enable_resources(dev, mask)) < 0)
> return err;
>
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> return 0;
> }
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index f211839e2cae..4a55d1b82d21 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -413,7 +413,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>
> if ((err = pci_enable_resources(dev, mask)) < 0)
> return err;
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> return 0;
> }
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index f5ec736100ee..7ccc64d5fe3e 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -398,7 +398,7 @@ pcibios_enable_device (struct pci_dev *dev, int mask)
> if (ret < 0)
> return ret;
>
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> return acpi_pci_irq_enable(dev);
> return 0;
> }
> @@ -407,7 +407,7 @@ void
> pcibios_disable_device (struct pci_dev *dev)
> {
> BUG_ON(atomic_read(&dev->enable_cnt));
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> acpi_pci_irq_disable(dev);
> }
>
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 3dfe2d31c67b..4d36ea517679 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -399,10 +399,12 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> {
> int err;
>
> - err = pci_enable_resources(dev, mask);
> - if (err = 0)
> + if ((err = pci_enable_resources(dev, mask)) < 0)
> + return err;
> +
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> - return err;
> + return 0;
> }
>
> /*
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-13 21:45 [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled Bjorn Helgaas
` (2 preceding siblings ...)
2018-03-14 10:27 ` Rafael J. Wysocki
@ 2018-03-14 16:31 ` Andy Shevchenko
2018-03-14 17:49 ` Bjorn Helgaas
2018-03-19 18:57 ` Bjorn Helgaas
4 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2018-03-14 16:31 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, Rafael J. Wysocki, Cris, Linux Kernel Mailing List,
linux-ia64, linux-am33-list
On Tue, Mar 13, 2018 at 11:45 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
Agree with Christoph's comment.
> If a device already has MSI or MSI-X enabled, there's no need to set up its
> legacy INTx interrupt.
Just point to the actual behaviour of this. In some cases code in
question has to distinguish between MSI and MSI-x.
So, this or similar changes has to be done with keeping above in mind.
(Existing example is Thunderbolt driver)
>
> bba6f6fc68e7 ("[PATCH] MSI-X: fix resume crash") changed the cris, frv,
> x86, and ia64 arches to skip INTx setup when MSI is enabled.
>
> 16cf0ebc35dd ("x86/PCI: Do not use interrupt links for devices using
> MSI-X") extended that by changing x86 to also skip INTx setup when MSI-X is
> enabled.
>
> Change the remaining arches (cris, frv, and ia64) to skip INTx setup when
> either MSI or MSI-X is enabled.
>
> Also update mn10300 (which didn't exist at the time of bba6f6fc68e7) to
> follow the same pattern.
Perhaps no need to change the architectures that are about to be
removed completely from the kernel.
FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> arch/cris/arch-v32/drivers/pci/bios.c | 2 +-
> arch/frv/mb93090-mb00/pci-vdk.c | 2 +-
> arch/ia64/pci/pci.c | 4 ++--
> arch/mn10300/unit-asb2305/pci.c | 8 +++++---
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
> index 6b9e6cfaa29e..c2bed0cc060b 100644
> --- a/arch/cris/arch-v32/drivers/pci/bios.c
> +++ b/arch/cris/arch-v32/drivers/pci/bios.c
> @@ -68,7 +68,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> if ((err = pcibios_enable_resources(dev, mask)) < 0)
> return err;
>
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> return 0;
> }
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index f211839e2cae..4a55d1b82d21 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -413,7 +413,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>
> if ((err = pci_enable_resources(dev, mask)) < 0)
> return err;
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> return 0;
> }
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index f5ec736100ee..7ccc64d5fe3e 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -398,7 +398,7 @@ pcibios_enable_device (struct pci_dev *dev, int mask)
> if (ret < 0)
> return ret;
>
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> return acpi_pci_irq_enable(dev);
> return 0;
> }
> @@ -407,7 +407,7 @@ void
> pcibios_disable_device (struct pci_dev *dev)
> {
> BUG_ON(atomic_read(&dev->enable_cnt));
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> acpi_pci_irq_disable(dev);
> }
>
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 3dfe2d31c67b..4d36ea517679 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -399,10 +399,12 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> {
> int err;
>
> - err = pci_enable_resources(dev, mask);
> - if (err = 0)
> + if ((err = pci_enable_resources(dev, mask)) < 0)
> + return err;
> +
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> - return err;
> + return 0;
> }
>
> /*
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-14 16:31 ` Andy Shevchenko
@ 2018-03-14 17:49 ` Bjorn Helgaas
2018-03-14 18:17 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2018-03-14 17:49 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-pci, Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, Rafael J. Wysocki, Cris, Linux Kernel Mailing List,
linux-ia64, linux-am33-list
On Wed, Mar 14, 2018 at 06:31:38PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 13, 2018 at 11:45 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
>
> Agree with Christoph's comment.
>
> > If a device already has MSI or MSI-X enabled, there's no need to set up its
> > legacy INTx interrupt.
>
> Just point to the actual behaviour of this.
By "point to the actual behaviour", do you mean adding something to
the changelog along the lines of the following?
If MSI or MSI-X is enabled, the device uses that. It uses INTx only
if both MSI and MSI-X are disabled (see PCIe r4.0, sec 7.7.1.2).
I did add that because I think that spec reference is useful. If you
have something else in mind, maybe an example would help me
understand.
> In some cases code in question has to distinguish between MSI and
> MSI-x. So, this or similar changes has to be done with keeping
> above in mind.
>
> (Existing example is Thunderbolt driver)
Sorry, I didn't get your point here. Certainly some code needs to
distinguish between MSI and MSI-X, but I don't think that's the case
here. I'm not proposing to change Thunderbolt; I do see that it uses
dev->msix_enabled (but not dev->msi_enabled), and it doesn't look like
using pci_dev_msi_enabled() there would be appropriate.
> > bba6f6fc68e7 ("[PATCH] MSI-X: fix resume crash") changed the cris, frv,
> > x86, and ia64 arches to skip INTx setup when MSI is enabled.
> >
> > 16cf0ebc35dd ("x86/PCI: Do not use interrupt links for devices using
> > MSI-X") extended that by changing x86 to also skip INTx setup when MSI-X is
> > enabled.
> >
> > Change the remaining arches (cris, frv, and ia64) to skip INTx setup when
> > either MSI or MSI-X is enabled.
> >
> > Also update mn10300 (which didn't exist at the time of bba6f6fc68e7) to
> > follow the same pattern.
>
> Perhaps no need to change the architectures that are about to be
> removed completely from the kernel.
Yeah, I figured there was a danger of that, but I haven't kept up on
exactly which are on the chopping block, so I figured it'd be trivial
to drop anything that turns out to be unnecessary.
> FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> > arch/cris/arch-v32/drivers/pci/bios.c | 2 +-
> > arch/frv/mb93090-mb00/pci-vdk.c | 2 +-
> > arch/ia64/pci/pci.c | 4 ++--
> > arch/mn10300/unit-asb2305/pci.c | 8 +++++---
> > 4 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
> > index 6b9e6cfaa29e..c2bed0cc060b 100644
> > --- a/arch/cris/arch-v32/drivers/pci/bios.c
> > +++ b/arch/cris/arch-v32/drivers/pci/bios.c
> > @@ -68,7 +68,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > if ((err = pcibios_enable_resources(dev, mask)) < 0)
> > return err;
> >
> > - if (!dev->msi_enabled)
> > + if (!pci_dev_msi_enabled(dev))
> > pcibios_enable_irq(dev);
> > return 0;
> > }
> > diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> > index f211839e2cae..4a55d1b82d21 100644
> > --- a/arch/frv/mb93090-mb00/pci-vdk.c
> > +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> > @@ -413,7 +413,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> >
> > if ((err = pci_enable_resources(dev, mask)) < 0)
> > return err;
> > - if (!dev->msi_enabled)
> > + if (!pci_dev_msi_enabled(dev))
> > pcibios_enable_irq(dev);
> > return 0;
> > }
> > diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> > index f5ec736100ee..7ccc64d5fe3e 100644
> > --- a/arch/ia64/pci/pci.c
> > +++ b/arch/ia64/pci/pci.c
> > @@ -398,7 +398,7 @@ pcibios_enable_device (struct pci_dev *dev, int mask)
> > if (ret < 0)
> > return ret;
> >
> > - if (!dev->msi_enabled)
> > + if (!pci_dev_msi_enabled(dev))
> > return acpi_pci_irq_enable(dev);
> > return 0;
> > }
> > @@ -407,7 +407,7 @@ void
> > pcibios_disable_device (struct pci_dev *dev)
> > {
> > BUG_ON(atomic_read(&dev->enable_cnt));
> > - if (!dev->msi_enabled)
> > + if (!pci_dev_msi_enabled(dev))
> > acpi_pci_irq_disable(dev);
> > }
> >
> > diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> > index 3dfe2d31c67b..4d36ea517679 100644
> > --- a/arch/mn10300/unit-asb2305/pci.c
> > +++ b/arch/mn10300/unit-asb2305/pci.c
> > @@ -399,10 +399,12 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > {
> > int err;
> >
> > - err = pci_enable_resources(dev, mask);
> > - if (err = 0)
> > + if ((err = pci_enable_resources(dev, mask)) < 0)
> > + return err;
> > +
> > + if (!pci_dev_msi_enabled(dev))
> > pcibios_enable_irq(dev);
> > - return err;
> > + return 0;
> > }
> >
> > /*
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-14 17:49 ` Bjorn Helgaas
@ 2018-03-14 18:17 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2018-03-14 18:17 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, Rafael J. Wysocki, Cris, Linux Kernel Mailing List,
linux-ia64, linux-am33-list
On Wed, Mar 14, 2018 at 7:49 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Mar 14, 2018 at 06:31:38PM +0200, Andy Shevchenko wrote:
>> On Tue, Mar 13, 2018 at 11:45 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> > If a device already has MSI or MSI-X enabled, there's no need to set up its
>> > legacy INTx interrupt.
>>
>> Just point to the actual behaviour of this.
>
> By "point to the actual behaviour", do you mean adding something to
> the changelog along the lines of the following?
>
> If MSI or MSI-X is enabled, the device uses that. It uses INTx only
> if both MSI and MSI-X are disabled (see PCIe r4.0, sec 7.7.1.2).
>
> I did add that because I think that spec reference is useful. If you
> have something else in mind, maybe an example would help me
> understand.
I meant that the behaviour now is changed from
check MSI only case
to
check MSI _or MSI-x_ case
Not all code paths may survive that.
>> In some cases code in question has to distinguish between MSI and
>> MSI-x. So, this or similar changes has to be done with keeping
>> above in mind.
>>
>> (Existing example is Thunderbolt driver)
>
> Sorry, I didn't get your point here. Certainly some code needs to
> distinguish between MSI and MSI-X, but I don't think that's the case
> here. I'm not proposing to change Thunderbolt; I do see that it uses
> dev->msix_enabled (but not dev->msi_enabled), and it doesn't look like
> using pci_dev_msi_enabled() there would be appropriate.
Exactly, that's why I pointed on above.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled
2018-03-13 21:45 [PATCH] PCI/MSI: Don't set up INTx if MSI or MSI-X is enabled Bjorn Helgaas
` (3 preceding siblings ...)
2018-03-14 16:31 ` Andy Shevchenko
@ 2018-03-19 18:57 ` Bjorn Helgaas
4 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2018-03-19 18:57 UTC (permalink / raw)
To: linux-pci
Cc: Mikael Starvik, Jesper Nilsson, Tony Luck, Fenghua Yu,
David Howells, Rafael J. Wysocki, linux-cris-kernel, linux-kernel,
linux-ia64, linux-am33-list
On Tue, Mar 13, 2018 at 04:45:49PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> If a device already has MSI or MSI-X enabled, there's no need to set up its
> legacy INTx interrupt.
>
> bba6f6fc68e7 ("[PATCH] MSI-X: fix resume crash") changed the cris, frv,
> x86, and ia64 arches to skip INTx setup when MSI is enabled.
>
> 16cf0ebc35dd ("x86/PCI: Do not use interrupt links for devices using
> MSI-X") extended that by changing x86 to also skip INTx setup when MSI-X is
> enabled.
>
> Change the remaining arches (cris, frv, and ia64) to skip INTx setup when
> either MSI or MSI-X is enabled.
>
> Also update mn10300 (which didn't exist at the time of bba6f6fc68e7) to
> follow the same pattern.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Applied to pci/msi with tested-by from Tony and reviewed-by Rafael and
Andy. And I added a note to my to-do list about getting rid of
pcibios_enable_device().
> ---
> arch/cris/arch-v32/drivers/pci/bios.c | 2 +-
> arch/frv/mb93090-mb00/pci-vdk.c | 2 +-
> arch/ia64/pci/pci.c | 4 ++--
> arch/mn10300/unit-asb2305/pci.c | 8 +++++---
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
> index 6b9e6cfaa29e..c2bed0cc060b 100644
> --- a/arch/cris/arch-v32/drivers/pci/bios.c
> +++ b/arch/cris/arch-v32/drivers/pci/bios.c
> @@ -68,7 +68,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> if ((err = pcibios_enable_resources(dev, mask)) < 0)
> return err;
>
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> return 0;
> }
> diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
> index f211839e2cae..4a55d1b82d21 100644
> --- a/arch/frv/mb93090-mb00/pci-vdk.c
> +++ b/arch/frv/mb93090-mb00/pci-vdk.c
> @@ -413,7 +413,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>
> if ((err = pci_enable_resources(dev, mask)) < 0)
> return err;
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> return 0;
> }
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index f5ec736100ee..7ccc64d5fe3e 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -398,7 +398,7 @@ pcibios_enable_device (struct pci_dev *dev, int mask)
> if (ret < 0)
> return ret;
>
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> return acpi_pci_irq_enable(dev);
> return 0;
> }
> @@ -407,7 +407,7 @@ void
> pcibios_disable_device (struct pci_dev *dev)
> {
> BUG_ON(atomic_read(&dev->enable_cnt));
> - if (!dev->msi_enabled)
> + if (!pci_dev_msi_enabled(dev))
> acpi_pci_irq_disable(dev);
> }
>
> diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
> index 3dfe2d31c67b..4d36ea517679 100644
> --- a/arch/mn10300/unit-asb2305/pci.c
> +++ b/arch/mn10300/unit-asb2305/pci.c
> @@ -399,10 +399,12 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> {
> int err;
>
> - err = pci_enable_resources(dev, mask);
> - if (err = 0)
> + if ((err = pci_enable_resources(dev, mask)) < 0)
> + return err;
> +
> + if (!pci_dev_msi_enabled(dev))
> pcibios_enable_irq(dev);
> - return err;
> + return 0;
> }
>
> /*
>
^ permalink raw reply [flat|nested] 9+ messages in thread