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
