Hi Krzysztof & Marc: Sorry for the interruption. As said before, we tried to use the 'interrupt-map attribute' in our Loongson liointc dts(i), but there are some unfriendly points. Do you have any other different suggestions? Thanks. Binbin On Wed, Oct 18, 2023 at 8:33 PM Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > Hi, Jonas, > > On Wed, Oct 18, 2023 at 5:38 PM Jonas Gorski <jonas.gorski@xxxxxxxxx> wrote: > > > > On Wed, 18 Oct 2023 at 08:58, Binbin Zhou <zhoubb.aaron@xxxxxxxxx> wrote: > > > > > > 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). > > > > 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? > Let me make things clear and ignore those irrelevant information. > 1, Our liointc controller has 32 inputs from downstream interrupt > sources and 4 outputs to the parent irqchip, the "routing" here means > which input go to which output. > 2, We use 'parent_int_map' to describe the boot-time routing in dts > previously, but Krzysztof suggests us to use 'interrupt-map' instead. > 3, When we rework our driver to use 'interrupt-map', we found that > this property is not supposed to be used in a regular irqchip (it is > usually used in a pcie port which is also act as an irqchip). > 4, If we still want to use 'interrupt-map' to describe the routing in > liointc, we should make of_irq_parse_raw() stop at the liointc level > rather than go to its parent level, because the hwirq is provided by > liointc, not its parent. To archive this goal, we should add liointc > to the quirk list. > 5, So, for our liointc driver we have two choices: 1) still use the > 'parent_int_map' property; 2) use 'interrupt-map' property and add > liointc to the quirk list. We prefer the first ourselves, but > Krzysztof may give us a best solution. > > Huacai > > > > > Best Regards, > > Jonas