Re: Similar SoCs with different CPUs and interrupt bindings

[Arnd Bergmann]

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