On 2014/9/17 1:43, Thomas Gleixner wrote:

Jiang,

On Thu, 11 Sep 2014, Jiang Liu wrote:

quoted

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. 	

quoted

 	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.

quoted

+#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.

quoted

+ * 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