On 05.09.2014 12:13, Arnd Bergmann wrote:
On Friday 05 September 2014 10:47:30 Marc Zyngier wrote:
I still prefer being explicit here for the same reason I mentioned earlier:
I want it to be very clear that we don't support arbitrary irqchips other
than the ones in the APCI specification. The infrastructure exists on DT
because we have to support a large number of incompatible irqchips.
I'm not suggesting that we should support more than the ACPI spec says.
And that's certainly the whole point of a spec, isn't it? ACPI says what
we support, and we're not going any further. I'm just saying that we
shouldn't make the maintenance burden heavier, and the code nothing
short of disgusting. Using our current infrastructure doesn't mean we're
going to support GIcv2.37.
Ok
In particular, the ACPI tables describing the irqchip have no way to
identify the GIC at all, if I read the spec correctly, you have to
parse the tables, ioremap the registers and then read the ID to know
if you have GICv1/v2/v2m/v3/v4. There doesn't seem to be any "device"
for the GIC that a hypothetical probe function would be based on.
This is not the way I read the spec. Table 5-46 (Interrupt Controller
Structure) tells you if you have a CPU interface (GICv1/v2) or a
redistributor (GICv3/v4). That's enough to know whether or not you
should carry on probing a particular controller.
Ah, good. I missed that.
The various GIC versions don't really have a unified memory map anyway
(well, none that you can rely on), and you really have to rely on ACPI
to tell you what you have.
So we are back to needing to support two different irqchip drivers
(v1/v2/v2m and v3/v4), instead of five or more, right?
It does seem wrong to parse the tables in the irq-gic.c file though:
that part can well be common across the various gic versions and then
call into either irq-gic.c or irq-gic-v3.c for the version specific
parts. Whether we put that common code into drivers/irqchip/irqchip.c,
drivers/irqchip/gic-common.c, drivers/irqchip/irq-acpi-gic.c or
drivers/acpi/irq-gic.c I don't care at all.
I don't think so you can make that common code very easily. The
information required by both drivers is organized differently.
If it was, I'd have done that for the DT binding.
I see, and that's also what Tomasz just explained. So can we just
have one an irqchip_init() function doing this:?
if (dt)
of_irq_init(__irqchip_of_table);
else if (acpi) {
read cpu-interface and redistributor address from acpi tables
if (cpu-interface)
gic_v2_acpi_init(table);
else if(redistributor)
gic_v3_acpi_init(table)
}
It is pretty much the same as:
if (dt)
of_irq_init(__irqchip_of_table);
else if (acpi) {
err = gic_v3_acpi_init(table) {
read redistributor address from acpi tables
[...]
}
if (err) {
err = gic_v2_acpi_init(table) {
read cpu-interface address from acpi tables
[...]
}
}
}
What basically is my current implementation (well, without GICv3 support
but that will be next step).
Tomasz
--
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
