On Sun, 2015-09-06 at 17:44 +0300, Michael S. Tsirkin wrote: > On Fri, Sep 04, 2015 at 08:17:12PM -0300, Guilherme G. Piccoli wrote: > > Hello Bjorn, > > > > >of_create_pci_dev() already has a lot of code that duplicates > > >pci_setup_device(), and it's a shame to add more. There's also a sparc > > >version of of_create_pci_dev() that presumably has the same problem you're > > >fixing for powerpc. > > > > Thanks for the information! > > > > >Michael originally called pci_msi_setup_pci_dev() from > > >pci_init_capabilities() [1]. A subsequent patch moved the call > > >to pci_setup_device() [2] because an early quirk (called from > > >pci_setup_device()) used pci_msi_off(), which depended on > > >pci_msi_setup_pci_dev(). > > > > > >But we later removed pci_msi_off() completely, so I think we probably > > >*could* call pci_msi_setup_pci_dev() from pci_init_capabilities(). > > > > > >That would be much nicer because it makes more sense there, and it > > >would do the right thing for powerpc and sparc because they both > > >already use that path. > > > > > >Can you look into moving the call? > > > > I might have misunderstood something here (sorry if it's the case), but > > moving the call to pci_init_capabilities() has the same practical > > implications than reverting my 2 commmits [1] [2] and Michael Tsirkin's > > commit [3], except when CONFIG_PCI_MSI is not set - in this case, moving the > > call would initialize MSI capabilities anyway, since pci_init_capabilities() > > executes even if CONFIG_PCI_MSI isn't set. > > > > My question is: is necessary to initialize MSI capabilities even with > > CONFIG_PCI_MSI not set? In negative case, would be "cleaner" revert the 3 > > commits, right? > > > > On the other hand, if it's necessary to initialize MSI capabilities on > > devices anyway, we can change the call place. > > I think the reason why it's necessary is explained in > commit log for commit 1851617cd2da9cc53cdc1738f4148f4f042c0e56 (that's > [3] below). Well yes and no. What we want to do when CONFIG_PCI_MSI=n is disable MSI on the device. In order to do that the code first initialises dev->msi[x]_cap. But arguably that's wrong, ie. when CONFIG_PCI_MSI=n dev->msi[x]_cap *should* be zero so that any code which erroneously tries to use them will fail. But perhaps that's being too pedantic :) cheers -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
