Re: [PATCH v2] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about liointc-2.0

[Binbin Zhou]

On Tue, Oct 17, 2023 at 9:05 PM Jonas Gorski <jonas.gorski@xxxxxxxxx> wrote:
>
> On Mon, 16 Oct 2023 at 13:26, Binbin Zhou <zhoubb.aaron@xxxxxxxxx> wrote:
> >
> > Hi all:
> >
> > Sorry, it's been a while since the last discussion.
> >
> > Previously, Krzysztof suggested using the standard "interrupt-map"
> > attribute instead of the "loongson,parent_int_map" attribute, which I
> > tried to implement, but the downside of this approach seems to be
> > obvious.
> >
> > First of all, let me explain again the interrupt routing of the
> > loongson liointc.
> > For example, the Loongson-2K1000 has 64 interrupt sources, each with
> > the following 8-bit interrupt routing registers (main regs attribute
> > in dts):
> >
> > +----+-------------------------------------------------------------------+
> > | bit  | description
> >             |
> > +----+-------------------------------------------------------------------+
> > | 3:0 | Processor core to route                                           |
> > | 7:4 | Routed processor core interrupt pins (INT0--INT3) |
> > +-----+------------------------------------------------------------------+
> >
> > The "loongson,parent_int_map" attribute is to describe the routed
> > interrupt pins to cpuintc.
> >
> > However, the "interrupt-map" attribute is not supposed to be used for
> > interrupt controller in the normal case. Though since commit
> > 041284181226 ("of/irq: Allow matching of an interrupt-map local to an
> > interrupt controller"), the "interrupt-map" attribute can be used in
> > interrupt controller nodes. Some interrupt controllers were found not
> > to work properly later, so in commit de4adddcbcc2 ("of/irq: Add a
> > quirk for controllers with their own definition of interrupt-map"), a
> > quirk was added for these interrupt controllers. As we can see from
> > the commit message, this is a bad solution in itself.
> >
> > Similarly, if we choose to use the "interrupt-map" attribute in the
> > interrupt controller, we have to use this unfriendly solution (quirk).
> > Because we hope of_irq_parse_raw() stops at the liointc level rather
> > than goto its parent level.
> >
> > So, I don't think it's a good choice to use a bad solution as a replacement.
> >
> > Do you have any other ideas?
>
> Assuming this is changeable at runtime, this sounds to me like this
> mapping/routing could easily be exposed as irqchip cpu affinity. Then
> userspace can apply all the performance optimizations it wants (and
> can easily update them without fiddling with the kernel/dts).
>
> And then there would be no need to hardcode/describe it in the dts(i) at all.

Hi Jonas:

Thanks for your reply.

It is possible that my non-detailed explanation caused your misunderstanding.
Allow me to explain again about the interrupt routing register above,
which we know is divided into two parts:

+----+-------------------------------------------------------------------+
| bit  | description |
+----+-------------------------------------------------------------------+
| 3:0 | Processor core to route                                           |
| 7:4 | Routed processor core interrupt pins (INT0--INT3) |
+-----+------------------------------------------------------------------+

The first part "processor core" will be set to "boot_cpu_id" in the
driver, which we assume is fixed and we don't need to care about it
here.
What we care about is the second part "mapping of device interrupts to
processor interrupt pins", which is what we want to describe in
dts(i).

Let's take the Loongson-2K1000 as an example again, it has 64
interrupt sources as inputs and 4 processor core interrupt pins as
outputs.
The sketch is shown below:

Device Interrupts           Interrupt Pins
                 +-------------+
         0---->|                |--> INT0
        ...       | Mapping |--> INT1
        ...       |                |--> INT2
        63--->|                |--> INT3
                 +-------------+

Therefore, this mapping relationship cannot be changed at runtime and
needs to be hardcoded/described in dts(i).

Thanks.
Binbin
>
> Best Regards,
> Jonas