Re: Similar SoCs with different CPUs and interrupt bindings

[Geert Uytterhoeven]

Hi Arnd,

On Thu, Sep 22, 2022 at 8:30 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Wed, Sep 21, 2022, at 11:20 AM, Robin Murphy wrote:
> > On 2022-09-21 08:46, Geert Uytterhoeven wrote:
> >> 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.

We also have to handle interrupt-parent at the /soc level.
And of course you never know what pops up next ;-)

> 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.

This is usually the case, but not always.
And the numbering may be the same (modulo the offset), but
not it really depends on the on-SoC wiring.

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

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds