Date:   Wed, 18 Oct 2023 11:38:09 +0200
From:   Jonas Gorski <jonas.gorski@...il.com>
To:     Binbin Zhou <zhoubb.aaron@...il.com>
Cc:     Marc Zyngier <maz@...nel.org>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Binbin Zhou <zhoubinbin@...ngson.cn>,
        Huacai Chen <chenhuacai@...ngson.cn>,
        Thomas Gleixner <tglx@...utronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Huacai Chen <chenhuacai@...nel.org>,
        loongson-kernel@...ts.loongnix.cn, devicetree@...r.kernel.org,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-mips@...r.kernel.org, diasyzhang@...cent.com,
        linux-kernel@...r.kernel.org, frowand.list@...il.com
Subject: Re: [PATCH v2] dt-bindings: interrupt-controller: loongson,liointc:
 Fix warnings about liointc-2.0

On Wed, 18 Oct 2023 at 08:58, Binbin Zhou <zhoubb.aaron@...il.com> wrote:
>
> On Tue, Oct 17, 2023 at 9:05 PM Jonas Gorski <jonas.gorski@...il.com> wrote:
> >
> > On Mon, 16 Oct 2023 at 13:26, Binbin Zhou <zhoubb.aaron@...il.com> 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).

But that's only a driver/description limitation, not an actual
physical limitation, right? In theory you could reroute them as much
as you want as long as you keep the kernel up-to-date about the
current routing (via whatever means).

Anyway, I guess you want to use the routed interrupt pin to identify
different irq controller blocks.

Can't the interrupt pin be inferred from the parent interrupt? If your
parent (hw) irq is two, route everything to INT0 etc? Or alternatively
use the name of the parent interrupt?

Best Regards,
Jonas

Powered by blists - more mailing lists