On 06/05/2020 10:16, Marc Zyngier wrote:
> On 2020-05-06 09:45, André Przywara wrote:
>> On 05/05/2020 19:25, Marc Zyngier wrote:
>>> On Tue, 05 May 2020 17:52:03 +0100,
>>> Andre Przywara <andre.przywara@xxxxxxx> wrote:
>>>>
>>>> The GIC DT binding only allows a certain combination of DT compatible
>>>> strings, mostly just consisting of one name.
>>>>
>>>> Drop the combination of multiple names and go with the
>>>> "arm,cortex-a15-gic" name for GICv2, as this seems to be the most
>>>> widely
>>>> accepted string. "arm,gic-400" would be more correct, but was
>>>> introduced
>>>> much later into the kernel's GIC driver.
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>>>> ---
>>>> arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi | 2 +-
>>>> arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
>>>> arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts | 2 +-
>>>> 3 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
>>>> b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
>>>> index 15fe81738e94..61a1750fcdd6 100644
>>>> --- a/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
>>>> +++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
>>>> @@ -6,7 +6,7 @@
>>>>
>>>> / {
>>>> gic: interrupt-controller@2c001000 {
>>>> - compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>>> + compatible = "arm,cortex-a15-gic";
>>>> #interrupt-cells = <3>;
>>>> #address-cells = <2>;
>>>> interrupt-controller;
>>>> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi
>>>> b/arch/arm64/boot/dts/arm/juno-base.dtsi
>>>> index 3feefd61eb76..62392ab1f880 100644
>>>> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
>>>> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
>>>> @@ -69,7 +69,7 @@
>>>> };
>>>>
>>>> gic: interrupt-controller@2c010000 {
>>>> - compatible = "arm,gic-400", "arm,cortex-a15-gic";
>>>> + compatible = "arm,cortex-a15-gic";
>>>
>>> Why? GIC-400 is definitely the most correct compatible string. I'd
>>> rather see this compatible being generalised to the models rather than
>>> only referencing the A15 GIC.
>>
>> I agree that gic-400 is the far better name, but it was only introduced
>> in v3.16. So omitting arm,cortex-a15-gic would break any kernels before
>> that, which I would like to avoid.
>
> I am not talking about dropping the A15 GIC. I'm saying that both should
> stay. Is there anything in the DT binding that forbids multiple names in
> the compatible property?
Well, the current form of the YAML bindings require every combination of
compatible strings to be listed, either explicitly, or using an list of
allowed strings for each position. This combination here is not listed
at the moment.
>> It's actually a pity that we are so picky about the compatible listings,
>> because the existing combination is actually quite nice: we get
>> compatibility with older DT consumers, but still can say what it
>> actually is.
>> I wonder if I should introduce this combination to the GIC DT binding
>> instead, it seems like there are other users in the tree as well.
>>
>> What do you think?
>
> I'd say that if the binding forbids multiple compatible strings, the
> binding is likely to be wrong. We should fix it, and not make the DTs
> worse as a result of a binding issue.
OK, thanks for the confirmation, and I agree. I will ditch this patch
and replace it with a respective bindings fix.
Thanks,
Andre.
