On Thu, Jan 17, 2013 at 08:30:10PM +0000, Thierry Reding wrote: > On Thu, Jan 17, 2013 at 04:22:18PM +0000, Andrew Murray wrote: > > On Thu, Jan 17, 2013 at 04:05:02PM +0000, Thierry Reding wrote: > > > On Thu, Jan 17, 2013 at 03:42:36PM +0000, Andrew Murray wrote: > > > > On Wed, Jan 16, 2013 at 06:31:01PM +0000, Thierry Reding wrote: > > > > > Alright, putting the functions into pci_ops doesn't sound like a very > > > > > good idea then. Or perhaps it would make sense for hardware where the > > > > > root complex and the MSI controller are handled by the same driver. > > > > > Basically it could be done as a shortcut and if those are not filled > > > > > in, the drivers could still opt to look up an MSI controller from a > > > > > phandle specified in DT. > > > > > > > > > > Even another alternative would be to keep the functions within the > > > > > struct pci_ops and use generic ones if an external MSI controller is > > > > > used. Just tossing around ideas. > > > > > > > > I think an ideal solution would be for additional logic in drivers/msi.c > > > > (e.g. in functions like msi_capability_init) to determine (based on the > > > > passed in pci_dev) which MSI controller ops to use. I'm not sure the best > > > > way to implement an association between an MSI controller and PCI busses > > > > (I believe arch/sparc does something like this - perhaps there will be > > > > inspiration there). > > > > > > > > As you've pointed out, most RCs will have their own MSI controllers - so > > > > it should be easy to register and associate both together. > > > > > > > > I've submitted my previous work on MSI controller registration, but it > > > > doesn't quite solve this problem - perhaps it can be a starting point? > > > > > > We basically have two cases: > > > > > > - The PCI host bridge contains registers for MSI support. In that case > > > it makes little sense to uncouple the MSI implementation from the > > > host bridge driver. > > > > > > - An MSI controller exists outside of the PCI host bridge. The PCI > > > host bridge would in that case have to lookup an MSI controller (via > > > DT phandle or some other method). > > > > > > In either of those cases, does it make sense to use the MSI support > > > outside the scope of the PCI infrastructure? That is, would devices > > > other than PCI devices be able to generate an MSI? > > > > I've come around to your way of thinking. Your approach sounds good for > > registration of MSI ops - let the RC host driver do it (it probably has its > > own), or use a helper for following a phandle to get ops that are not part > > of the driver. MSIs won't be used outside of PCI devices. > > > > Though existing drivers will use MSI framework functions to request MSIs, that > > will result in callbacks to the arch_setup_msi_irqs type functions. These > > functions would need to be updated to find these new ops if they exist, i.e. by > > traversing the pci_dev structure up to the RC and finding a suitable structure. > > > > Perhaps the msi ops could live alongside pci_ops in the pci_bus structure. This > > way when traversing up the buses from the provided pci_dev - the first bus with > > msi ops populated would be used? > > > > If no ops are found, the standard arch callbacks can be called - thus preserving > > exiting functionality. > > Yes, what you describe is exactly what I had in mind. I've been thinking > about a possible implementation and there may be some details that could > prove difficult to resolve. For instance, we likely need to pass context > around for the MSI ops, or else make sure that they can find the context > from the struct pci_dev or by traversing upwards from it. > > I think for the case where the MSI hardware is controlled by the same > driver as the PCI host bridge, doing this is easy because the context > could be part of the PCI host bridge context, which in case of Tegra is > stored in struct pci_bus' sysdata field (which is actually an ARM struct > pci_sys_data and in turn stores a pointer to the struct tegra_pcie in > the .private_data field). Other drivers often just use a global variable > assuming that there will only ever be a single instance of the PCI host > bridge. Yes. > If the MSI controller is external to the PCI host bridge, things get a > little more complicated. The easiest way would probably be to store the > context along with the PCI host bridge context and use simple wrappers > around the actual implementations to retrieve the PHB context and pass > the attached MSI context. This would be nice, but may not be necessary. The MSI controller could use a global (file-scope) variable to hold context gained from its probe and assume it will be the only instance of that MSI controller. > > Maybe this could even be made more generic by adding a struct msi_ops * > along with a struct msi_chip * in struct pci_bus. Perhaps I should try > and code something up to make things more concrete. This would be the most complete way to handle this. Andrew Murray -- 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