On Mon, 3 Nov 2014, Suravee Suthikulanit wrote: > On 11/3/2014 4:51 PM, Thomas Gleixner wrote: > > On Mon, 3 Nov 2014, suravee.suthikulpanit@xxxxxxx wrote: > > > + irq_domain_set_hwirq_and_chip(v2m->domain, virq, hwirq, > > > + &v2m_chip, v2m); > > > + > > > + irq_set_msi_desc(hwirq, desc); > > > + irq_set_irq_type(hwirq, IRQ_TYPE_EDGE_RISING); > > > > Sure both calls work perfectly fine as long as virq == hwirq, right? > > I was running into an issue when calling the irq_domain_alloc_irq_parent(), it > requires of_phandle_args pointer to be passed in. However, this does not work > for GICv2m since it does not have interrupt information in the device tree. > So, I decided at first to use direct (virq == hwirq) mapping, which simplifies > the code a bit, but might not be ideal solution, as you pointed out. It's not only far from ideal. It's not a solution at all. Simply because there is no guarantee for virq == hwirq. > An alternative would be to create a temporary struct of_phandle_args, and > populate it with the interrupt information for the requested MSI. Then pass it > to: > --> irq_domain_alloc_irq_parent > |--> gic_irq_domain_alloc > |--> gic_irq_domain_xlate > |--> gic_irq_domain_map > > However, this would still not be ideal if we want to support ACPI. Another Neither device tree nor ACPI has anything to do with MSI interrupts at runtime. All they do is to tell that there is a MSI controller and where the registers are and in the worst case fixups for a borked MSI_TYPER register. So either the TYPER reg or DT/ACPI gives you a fixed hwirq range which is reserved for MSI. And that's all you need, right? The MSI interrupt itself has no DT/ACPI information to use at allocation time simply because you CANNOT decribe a MSI device interrupt in DT/ACPI by any means. And you do not need any DT/ACPI information at that point. All you need is to pick one hwirq out of the existing fixed range and associate it to a newly allocated virq. That's the only information the underlying gic domain has to know about, because it needs to translate from the hwirq to the virq in the low level entry handler gic_handle_irq(). > alternative would be coming up with a dedicate structure to be used here. I > noticed on X86, it uses struct irq_alloc_info. May be that's what we also need > here. It's a x86 concept to transport X86 specific information in order to avoid duplicated code all over the place. And x86 MSI support is a completely different beast than the thing you are dealing with. x86 has no concept of a fixed hwirq range for MSI. So no, just picking random stuff from random MSI implementations does not help at all. > > [...] > > I do not care at all how YOU waste your time. But I care very much > > about the fact that YOU are wasting MY precious time by exposing me to > > your patch trainwrecks. > > I don't intend to waste yours or anybody's precious time. Sorry if it takes a > couple iterations to work out the issues. Also, I will try to put more comment > in my code to make it more clear. Let me know what works best for you to work > out the issues. By not sending obviously broken and half thought out patches in the first place. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
