On Tue, Jan 27, 2015 at 04:12:08PM +0000, Grant Likely wrote:
On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier marc.zyngier@arm.com wrote:
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
- if (!handle_arch_irq)
acpi_gic_init();Why isn't this called from irqchip_init? It would seem like the logical spot to probe an interrupt controller.
What has been done here isn't an unusual choice. We've got stuff all over the kernel that may or may not be implemented depending on what the architecture supports. If the function call is renamed to acpi_init_irq(), are you content?
My (full) suggestion was to do it like we've done it for DT, and I don't think I varied much from this point of view. Yes, calling it acpi_irq_init() would be a good start, and having the ACPI-compatible irqchips to be self-probable even better.
<lack-of-sleep-rant>
Hell, if nobody beats me to it, maybe I'll just write that code, with proper entry points in the various GIC drivers. Yes, this is infrastructure. Maybe it is grossly overdesigned. But I really spend too much time dealing with the crap that people are happy to pile on top of the GIC code to be madly enthusiastic about the general "good enough" attitude.
</lack-of-sleep-rant>
Or to put it in a slightly more diplomatic way: If ACPI is to be our future, can we please make the future look a bit better?
Hi Marc,
As per our off-list discussion, I completely agree. We don't want to be adding hack upon hack, and I will be first in line to NAK any patches taking that approach. However, for this initial series, it only supports exactly one GIC that can be set up by ACPI. Can we agree to leave it as is in this series, with the agreement that it will be replaced for v2m and v3 support with a proper pluggable initializer?
Can we at least call it acpi_init_irq() and avoid #including gic-specific header files? IOW hide the apci_gic_init() behind some generically named macro until the full solution is in place.