Dear Arnd Bergmann, On Tue, 26 Mar 2013 21:10:15 +0000, Arnd Bergmann wrote: > > To which children? Only to the main-intc children? If so, > > armada_370_xp_mpic_of_init() would be called with a 'device_node *' > > that points to the main-intc, correct? Then it would have to go back up > > in the Device Tree to find the msi node? It's doable of course, but > > sounds strange, no? > > I was thinking of registering two init functions as well. But then which of the two init functions would do the of_iomap() (or whatever ioremap()ing function you use) ? Remember, the very reason what we have *one* driver for both interrupt controllers is because the registers are completely mixed. So to me it's really the case of *one* device providing *two* features, like a device that would be both an Ethernet interface and a SPI controller. You have a single ->probe() function that gets called when the device is detected, and this ->probe() function registers both an Ethernet interface and a SPI controller. To me, the case we have here is really identical: we have one single set of registers that provide multiple features. > > To me, the topology of the hardware is really that a single device > > provides two features: the main interrupt controller and the MSI > > interrupt controller. But I will adapt to whatever DT binding you > > propose. > > My preference would be to have no sub-nodes but rather make the > code deal with an interrupt controller that has an interrupt domain > for regular IRQs but can also handle MSI. Hum, ok. > > > I still wonder if the real solution shouldn't instead be to make the > > > irq domain code MSI aware. For instance, you don't really need a > > > cell to describe an interrupt because the interrupt number is > > > not a hardware property. So an MSI using device doesn't really > > > needs an "msis" or "interrupts" property, just an "msi-parent", > > > and we can add code to handle as a separate domain even if you > > > have a single device node that can do both level and message > > > interrupts. > > > > So the msi-parent property should point to the single mpic node? But > > then the IRQ domain for MSI cannot be registered on this single MPIC > > node. Then, what would be the first argument of: > > > > armada_370_xp_msi_domain = > > irq_domain_add_linear(msi_node, PCI_MSI_DOORBELL_NR, > > &armada_370_xp_msi_irq_ops, NULL); > > > > and how would the PCIe driver get a pointer to this IRQ domain? (In the > > currently proposed code, it just does a irq_find_host(), which sounded > > very simple and straightforward). > > I guess one way would be to have a single domain that is responsible > for the controller and handles both MSI and legacy interrupts. That > could probably be done without changes to the interrupt domain code. > > Another option would be to add an irq_domain_add_msi() function that > creates a domain which is also tied to the same device node but does > not interact with it when going through the legacy interrupts. > You could then add a matching msi_find_host() or irq_find_msi_host() > function to return the domain. This option seems doable. Not sure yet how to do it exactly, but at least I understand the idea. > > To clarify: I really don't mind reworking the code, but unfortunately > > "make the IRQ domain code MSI aware" is too vague for me to understand > > the direction you're thinking of. > > I don't have specific code in mind yet, mainly playing around with the > possibilities for now. Sure, I understand. But I also don't have specific ideas: the current code works fine for me, and I don't find it really ugly. So if you don't point me in the direction that you think would make the code less ugly, then I'm lost. Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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
