[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <54099550.5080902@linaro.org>
Date: Fri, 05 Sep 2014 12:49:52 +0200
From: Tomasz Nowicki <tomasz.nowicki@...aro.org>
To: Marc Zyngier <marc.zyngier@....com>, Arnd Bergmann <arnd@...db.de>
CC: "linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
Catalin Marinas <Catalin.Marinas@....com>,
Liviu Dudau <Liviu.Dudau@....com>,
Lv Zheng <lv.zheng@...el.com>, Rob Herring <robh@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Robert Moore <robert.moore@...el.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
Robert Richter <rric@...nel.org>,
Jason Cooper <jason@...edaemon.net>,
Will Deacon <Will.Deacon@....com>,
Mark Brown <broonie@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Randy Dunlap <rdunlap@...radead.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Olof Johansson <olof@...om.net>
Subject: Re: [Linaro-acpi] [PATCH v3 13/17] ARM64 / ACPI: Add GICv2 specific
ACPI boot support
On 05.09.2014 12:39, Marc Zyngier wrote:
> On 05/09/14 11: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?
>
> As long as we make sure that the variants are directly probed from the
> "canonical" driver (v2m from v2, ITS and v4 from v3), then we're OK.
Yes, that is our intention.
>
>>>> 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)
>> }
>
> That'd be better, but that only considers the basic architecture. GICv2m
> and GICv3's ITS bring additional complexity (and their own table
> parsing). At that stage, I'm not sure how much commonality we're left with.
Right, my local ACPI GICv3 support assumes to probe ITS from
gic_v3_acpi_init() body.
Tomasz
--
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