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-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
