[PATCH] Enable MSI/MSI-X caps and disable MSI interrupts at PCI probe time - code move

[Guilherme G. Piccoli]

On 11/30/2015 11:35 PM, Bjorn Helgaas wrote:
> Thanks for your patience, Guilherme.

Hello Bjorn, thank you for the very deep review of my patch. Much 
appreciated!


> I applied this to pci/msi for v4.5.  I reworked the changelog and made
> pci_msi_setup_pci_dev() static, since I think it's now only called
> from drivers/pci/probe.c.

Thanks. Regarding pci_msi_setup_pci_dev(), it really should be static. I 
changed it because PowerPC arch was calling this since my last patch, 
but this patch remove the call from PowerPC arch, so it's great to see 
this function static again.


> I didn't quite follow the last paragraph about reverting 1851617cd2da.
> 1851617cd2da moved the MSI init (dev->msi_cap init and MSI disable)
> from pci/msi.c (where it was only done when CONFIG_PCI_MSI=y) to
> pci/probe.c (where we did it for all non-OF arches, regardless of
> CONFIG_PCI_MSI).  So I would characterize this patch as doing the MSI
> init for *all* arches, regardless of CONFIG_PCI_MSI.

Well, my English skills aren't so good, so I possibly I wrote confusing 
text. What I meant is that, from practical viewpoint, what we did here 
has the same effect of reverting those patches (in the sense of 
initializing MSI caps). But you can ignore my statement, and I 
appreciate that you removed it from the commit message if it was 
confusing heh


> While looking at this, I noticed that pci_msi_init_pci_dev() is now
> always empty, so I'll remove that in a separate patch.
>
> Please let me know if this doesn't make sense:

The patch below is great, thanks for the modifications and for removing 
pci_msi_init_pci_dev() in a separated patch.


> commit e80e7edc55ba711f3fe23975061b3f1c336ceb95
> Author: Guilherme G. Piccoli <gpiccoli at linux.vnet.ibm.com>
> Date:   Wed Oct 21 12:17:35 2015 -0200
>
>      PCI/MSI: Initialize MSI capability for all architectures
>
>      1851617cd2da ("PCI/MSI: Disable MSI at enumeration even if kernel doesn't
>      support MSI") moved dev->msi_cap and dev->msix_cap initialization from the
>      pci_init_capabilities() path (used on all architectures) to the
>      pci_setup_device() path (not used on Open Firmware architectures).
>
>      This broke MSI or MSI-X on Open Firmware machines.  4d9aac397a5d
>      ("powerpc/PCI: Disable MSI/MSI-X interrupts at PCI probe time in OF case")
>      fixed it for PowerPC but not for SPARC.
>
>      Set up MSI and MSI-X (initialize msi_cap and msix_cap and disable MSI and
>      MSI-X) in pci_init_capabilities() so all architectures do it the same way.
>
>      This reverts 4d9aac397a5d since this patch fixes the problem generically
>      for both PowerPC and SPARC.
>
>      [bhelgaas: changelog, make pci_msi_setup_pci_dev() static]
>      Fixes: 1851617cd2da ("PCI/MSI: Disable MSI at enumeration even if kernel doesn't support MSI")
>      Signed-off-by: Guilherme G. Piccoli <gpiccoli at linux.vnet.ibm.com>
>      Signed-off-by: Bjorn Helgaas <bhelgaas at google.com>
>
> 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 edb1984..cd94737 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1145,7 +1145,7 @@ int pci_cfg_space_size(struct pci_dev *dev)
>
>   #define LEGACY_IO_RESOURCE	(IORESOURCE_IO | IORESOURCE_PCI_FIXED)
>
> -void pci_msi_setup_pci_dev(struct pci_dev *dev)
> +static void pci_msi_setup_pci_dev(struct pci_dev *dev)
>   {
>   	/*
>   	 * Disable the MSI hardware to avoid screaming interrupts
> @@ -1212,8 +1212,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 */
> @@ -1606,6 +1604,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);
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e828e7b..f9f79ad 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1248,8 +1248,6 @@ struct msix_entry {
>   	u16	entry;	/* driver uses to specify entry, OS writes */
>   };
>
> -void pci_msi_setup_pci_dev(struct pci_dev *dev);
> -
>   #ifdef CONFIG_PCI_MSI
>   int pci_msi_vec_count(struct pci_dev *dev);
>   void pci_msi_shutdown(struct pci_dev *dev);
>

Cheers,


Guilherme Piccoli