[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <541A89AC.9000706@linux.intel.com>
Date: Thu, 18 Sep 2014 15:28:44 +0800
From: Jiang Liu <jiang.liu@...ux.intel.com>
To: Thomas Gleixner <tglx@...utronix.de>
CC: Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Randy Dunlap <rdunlap@...radead.org>,
Yinghai Lu <yinghai@...nel.org>,
Borislav Petkov <bp@...en8.de>,
Grant Likely <grant.likely@...aro.org>,
Marc Zyngier <marc.zyngier@....com>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Tony Luck <tony.luck@...el.com>,
Joerg Roedel <joro@...tes.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
x86@...nel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC Part2 v1 01/21] irqdomain: Introduce new interfaces to support
hierarchy irqdomains
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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists