| Message ID | 1445437055-7017-1-git-send-email-gpiccoli@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
Bjorn, ping? -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/21/2015 12:17 PM, Guilherme G. Piccoli wrote: > Commit 1851617cd2da ("PCI/MSI: Disable MSI at enumeration even if kernel > doesn't support MSI") changed the location of the code that initializes > dev->msi_cap/msix_cap and disables MSI/MSI-X interrupts at PCI probe > time in devices that have this flag set. It moved the code from > pci_msi_init_pci_dev() to a new function named pci_msi_setup_pci_dev(), > called by pci_setup_device(). > > In Open Firmware code path (PowerPC pSeries/SPARC archs) the function > pci_setup_device() is not called, so MSI capabilities are never enabled, > leading to error messages as: > > bnx2x 0000:01:00.0: no msix capability found > > Commit 4d9aac397a5d ("powerpc/PCI: Disable MSI/MSI-X interrupts at PCI > probe time in OF case") solved the issue on PowerPC pSeries arch calling > manually pci_msi_setup_pci_dev() on appropriate place. However, this > modification does not solve the general case (SPARC arch should be > modified too) and duplicates a lot of code, as pointed by Bjorn Helgaas. > As suggested by him, worth to reorganize the code to generally solve the > MSI caps issue and avoid too much code duplication. > > This patch does exactly this: we remove both the pci_msi_setup_pci_dev() > call from pci_setup_device() and the same call in OF code path of PowerPC > pSeries arch. Then, we call pci_msi_setup_pci_dev() directly from > pci_init_capabilities(). So, we can initialize MSI caps and disable MSI > interruptions during PCI probe in general fashion, avoiding code > duplication. > > Notice that this patch has the same practical effect of reverting > commit 1851617cd2da ("PCI/MSI: Disable MSI at enumeration even if kernel > doesn't support MSI") and commit 4d9aac397a5d ("powerpc/PCI: Disable > MSI/MSI-X interrupts at PCI probe time in OF case"). Regarding the > former, the author called pci_msi_setup_pci_dev() from pci_setup_device() > because there was an early quirk used in pci_msi_off(), which depended on > pci_msi_setup_pci_dev(). Since pci_msi_off() was completely removed by > commit c6201cd8513d ("PCI/MSI: Remove unused pci_msi_off()"), we can call > pci_msi_setup_pci_dev() directly from pci_init_capabilities(). > > Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/pci_of_scan.c | 3 --- > drivers/pci/probe.c | 5 +++-- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c > index 2e710c1..526ac67 100644 > --- a/arch/powerpc/kernel/pci_of_scan.c > +++ b/arch/powerpc/kernel/pci_of_scan.c > @@ -187,9 +187,6 @@ struct pci_dev *of_create_pci_dev(struct device_node *node, > > pci_device_add(dev, bus); > > - /* Setup MSI caps & disable MSI/MSI-X interrupts */ > - pci_msi_setup_pci_dev(dev); > - > return dev; > } > EXPORT_SYMBOL(of_create_pci_dev); > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 8361d27..0aac86e 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1209,8 +1209,6 @@ int pci_setup_device(struct pci_dev *dev) > /* "Unknown power state" */ > dev->current_state = PCI_UNKNOWN; > > - pci_msi_setup_pci_dev(dev); > - > /* Early fixups, before probing the BARs */ > pci_fixup_device(pci_fixup_early, dev); > /* device class may be changed after fixup */ > @@ -1600,6 +1598,9 @@ static void pci_init_capabilities(struct pci_dev *dev) > /* MSI/MSI-X list */ > pci_msi_init_pci_dev(dev); > > + /* Setup MSI caps & disable MSI/MSI-X interrupts */ > + pci_msi_setup_pci_dev(dev); > + > /* Buffers for saving PCIe and PCI-X capabilities */ > pci_allocate_cap_save_buffers(dev); > Michael, Bjorn: any news on this one? Thanks, Guilherme -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 04, 2015 at 10:00:15AM -0200, Guilherme G. Piccoli wrote:
> Bjorn, ping?
Sorry, Guilherme, this dropped off my radar, and I can't remember why.
I'll take a look at it soon.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/24/2015 07:23 PM, Bjorn Helgaas wrote: > On Wed, Nov 04, 2015 at 10:00:15AM -0200, Guilherme G. Piccoli wrote: >> Bjorn, ping? > > Sorry, Guilherme, this dropped off my radar, and I can't remember why. > I'll take a look at it soon. > > Bjorn > Thanks very much Bjorn! Sorry to bother, by the way. Cheers, Guilherme -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c index 2e710c1..526ac67 100644 --- a/arch/powerpc/kernel/pci_of_scan.c +++ b/arch/powerpc/kernel/pci_of_scan.c @@ -187,9 +187,6 @@ struct pci_dev *of_create_pci_dev(struct device_node *node, pci_device_add(dev, bus); - /* Setup MSI caps & disable MSI/MSI-X interrupts */ - pci_msi_setup_pci_dev(dev); - return dev; } EXPORT_SYMBOL(of_create_pci_dev); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 8361d27..0aac86e 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1209,8 +1209,6 @@ int pci_setup_device(struct pci_dev *dev) /* "Unknown power state" */ dev->current_state = PCI_UNKNOWN; - pci_msi_setup_pci_dev(dev); - /* Early fixups, before probing the BARs */ pci_fixup_device(pci_fixup_early, dev); /* device class may be changed after fixup */ @@ -1600,6 +1598,9 @@ static void pci_init_capabilities(struct pci_dev *dev) /* MSI/MSI-X list */ pci_msi_init_pci_dev(dev); + /* Setup MSI caps & disable MSI/MSI-X interrupts */ + pci_msi_setup_pci_dev(dev); + /* Buffers for saving PCIe and PCI-X capabilities */ pci_allocate_cap_save_buffers(dev);
Commit 1851617cd2da ("PCI/MSI: Disable MSI at enumeration even if kernel doesn't support MSI") changed the location of the code that initializes dev->msi_cap/msix_cap and disables MSI/MSI-X interrupts at PCI probe time in devices that have this flag set. It moved the code from pci_msi_init_pci_dev() to a new function named pci_msi_setup_pci_dev(), called by pci_setup_device(). In Open Firmware code path (PowerPC pSeries/SPARC archs) the function pci_setup_device() is not called, so MSI capabilities are never enabled, leading to error messages as: bnx2x 0000:01:00.0: no msix capability found Commit 4d9aac397a5d ("powerpc/PCI: Disable MSI/MSI-X interrupts at PCI probe time in OF case") solved the issue on PowerPC pSeries arch calling manually pci_msi_setup_pci_dev() on appropriate place. However, this modification does not solve the general case (SPARC arch should be modified too) and duplicates a lot of code, as pointed by Bjorn Helgaas. As suggested by him, worth to reorganize the code to generally solve the MSI caps issue and avoid too much code duplication. This patch does exactly this: we remove both the pci_msi_setup_pci_dev() call from pci_setup_device() and the same call in OF code path of PowerPC pSeries arch. Then, we call pci_msi_setup_pci_dev() directly from pci_init_capabilities(). So, we can initialize MSI caps and disable MSI interruptions during PCI probe in general fashion, avoiding code duplication. Notice that this patch has the same practical effect of reverting commit 1851617cd2da ("PCI/MSI: Disable MSI at enumeration even if kernel doesn't support MSI") and commit 4d9aac397a5d ("powerpc/PCI: Disable MSI/MSI-X interrupts at PCI probe time in OF case"). Regarding the former, the author called pci_msi_setup_pci_dev() from pci_setup_device() because there was an early quirk used in pci_msi_off(), which depended on pci_msi_setup_pci_dev(). Since pci_msi_off() was completely removed by commit c6201cd8513d ("PCI/MSI: Remove unused pci_msi_off()"), we can call pci_msi_setup_pci_dev() directly from pci_init_capabilities(). Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com> --- arch/powerpc/kernel/pci_of_scan.c | 3 --- drivers/pci/probe.c | 5 +++-- 2 files changed, 3 insertions(+), 5 deletions(-)