On 2014/9/17 1:43, Thomas Gleixner wrote: > Jiang, > > On Thu, 11 Sep 2014, Jiang Liu wrote: >> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c >> /* Create mapping */ >> - virq = irq_create_mapping(domain, hwirq); >> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY >> + if (domain->ops->alloc) >> + virq = irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE, >> + irq_data); >> + else >> +#endif >> + virq = irq_create_mapping(domain, hwirq); > > I'd prefer to get rid of the #ifdef CONFIG...s in the code. So this > can be written: > > if (irq_domain_has_hierarchy(domain)) > virq = irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE, > irq_data); > else > virq = irq_create_mapping(domain, hwirq); Sure, will kill the ifdef. > > >> if (!virq) >> return virq; >> >> @@ -540,7 +542,11 @@ unsigned int irq_find_mapping(struct irq_domain *domain, >> return 0; >> >> if (hwirq < domain->revmap_direct_max_irq) { >> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY >> + data = irq_domain_get_irq_data(domain, hwirq); >> +#else >> data = irq_get_irq_data(hwirq); >> +#endif > > Similar here. Make irq_domain_get_irq_data() map to irq_get_irq_data() for > the non hierarchy mode so you end up with a single line: > > - data = irq_get_irq_data(hwirq); > + data = irq_domain_get_irq_data(domain, hwirq); Sure. >> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY >> +/** >> + * irq_domain_alloc_irqs - Allocate IRQs from domain >> + * @domain: domain to allocate from >> + * @irq_base: allocate specified IRQ nubmer if irq_base >= 0 >> + * @nr_irqs: number of IRQs to allocate >> + * @node: NUMA node id for memory allocation >> + * @arg: domain specific argument >> + * @realloc: IRQ descriptors have already been allocated if true >> + * >> + * Allocate IRQ numbers and initialized all data structures to support >> + * hiearchy IRQ domains. >> + * Parameter @realloc is mainly to support legacy IRQs. > > What's the issue with the legacy irqs? So this has the interrupt > descriptors allocated already. Are they already wired up for serving > interrupts and what's the state of those lines? Function arch_early_ioapic_init() will allocate irq descriptors and irq_cfg structures for all legacy IRQ for three purposes: 1) To support ISA IRQs managed by 8259. 2) To reserve vectors on all CPUs for legacy IRQs 3) Prepare data structures to support pre_init_apic_IRQ0(). We will kill pre_init_apic_IRQ0() soon, so item 3 above won't be needed anymore. When __irq_domain_alloc_irqs() is called, only irq descriptor and irq_cfg have been allocated, but the interrupt controller hardware should be untouched yet. > >> + * Returns error code or allocated IRQ number > > Can you please add some documentation how the hierarchical allocation > is supposed to work and how the domains are connected. That should > probably go to Documentation/IRQ-domains.txt. Sure, I will do my best to add documentations for it. > > Other than that this looks pretty good! Nice work! Thanks! Gerry > > Thanks, > > tglx > -- 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