Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-935-ge4ccd4c47b-fm-20220914.001-ge4ccd4c4
Mime-Version: 1.0
Message-Id: <0603b2a5-6253-4c4b-8b30-aa0253ed0480@www.fastmail.com>
In-Reply-To: <45d2e6c2-3b4b-5720-0431-002c74b1f9cc@arm.com>
References: <CAMuHMdUPm36RsxHdVwspR3NCAR3C507AyB6R65W42N2gXWq0ag@mail.gmail.com>
 <45d2e6c2-3b4b-5720-0431-002c74b1f9cc@arm.com>
Date:   Thu, 22 Sep 2022 08:30:26 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Robin Murphy" <robin.murphy@arm.com>,
        "Geert Uytterhoeven" <geert@linux-m68k.org>,
        "Rob Herring" <robh+dt@kernel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>
Cc:     "Andre Przywara" <andre.przywara@arm.com>,
        "Conor.Dooley" <conor.dooley@microchip.com>,
        "Samuel Holland" <samuel@sholland.org>,
        "Biju Das" <biju.das.jz@bp.renesas.com>,
        "Chris Paterson" <Chris.Paterson2@renesas.com>,
        "Atish Patra" <atishp@atishpatra.org>,
        "Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        "Linux ARM" <linux-arm-kernel@lists.infradead.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: Similar SoCs with different CPUs and interrupt bindings
Content-Type: text/plain
Precedence: bulk

On Wed, Sep 21, 2022, at 11:20 AM, Robin Murphy wrote:
> On 2022-09-21 08:46, Geert Uytterhoeven wrote:
>>          Hi Rob, Krzysztof,
>> 
>> This is a topic that came up at the RISC-V BoF at Plumbers, and it was
>> suggested to bring it up with you.
>> 
>> The same SoC may be available with either RISC-V or other (e.g. ARM) CPU
>> cores (an example of this are the Renesas RZ/Five and RZ/G2UL SoCs).
>> To avoid duplication, we would like to have:
>>    - <riscv-soc>.dtsi includes <base-soc>.dtsi,
>>    - <arm-soc>.dtsi includes <base-soc>.dtsi.
>> 
>> Unfortunately RISC-V and ARM typically use different types of interrupt
>> controllers, using different bindings (e.g. 2-cell vs. 3-cell), and
>> possibly using different interrupt numbers.  Hence the interrupt-parent
>> and interrupts{-extended} properties should be different, too.
>> 
>> Possible solutions[1]:
>>    1. interrupt-map
>> 
>>    2. Use a SOC_PERIPHERAL_IRQ() macro in interrupts properties in
>>       <base-soc>.dtsi, with
>>         - #define SOC_PERIPHERAL_IRQ(nr, na) nr          // RISC-V
>>         - #define SOC_PERIPHERAL_IRQ(nr, na) GIC_SPI na  // ARM
>>       Note that the cpp/dtc combo does not support arithmetic, so even
>>       the simple case where nr = 32 + na cannot be simplified.
>> 
>>    3. Wrap inside RISCV() and ARM() macros, e.g.:
>> 
>>          RISCV(interrupts = <412 IRQ_TYPE_LEVEL_HIGH>;)
>>          ARM(interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;)
>> 
>>       Cfr. ARM() and THUMB() in arch/arm/include/asm/unified.h, as used
>>       to express the same operation using plain ARM or ARM Thumb
>>       instructions.
>
> 4. Put all the "interrupts" properties in the SoC-specific DTSI at the 
> same level as the interrupt controller to which they correspond. Works 
> out of the box with no horrible mystery macros, and is really no more or 
> less error-prone than any other approach. Yes, it means replicating a 
> bit of structure and/or having labels for everything (many of which may 
> be wanted anyway), but that's not necessarily a bad thing for 
> readability anyway. Hierarchical definitions are standard FDT practice 
> and should be well understood, so this is arguably the simplest and 
> least surprising approach :)

FWIW, approaches 1, 2 and 4 all seem reasonable to me, but I don't
like number 3 if this is only about the IRQ definitions.

It sounds like we're already converging on #2, so just one more
idea from me: we could fold the IRQ type into the macro, and
make it just take a single argument for extra flexibility:

#define SOC_PERIPHERAL_IRQ_LEVEL_HIGH(nr) \
        GIC_SPI (nr + offset) IRQ_TYPE_LEVEL_HIGH

If all the irqs on the chip have the same type, the name
can be shorter of course.

Either way, some variation of the macro sounds like a good enough
approach to me.

     Arnd