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

From: Binbin Zhou
Date: Mon Oct 16 2023 - 07:26:37 EST


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?

Thanks.
Binbin

On Mon, Sep 4, 2023 at 2:54 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> On Wed, 30 Aug 2023 16:25:48 +0100,
> Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote:
> >
> >
> >
> > 在 2023/8/30 21:44, Marc Zyngier 写道:
> > [...]
> > >> What's the best way, in your opinion, to overhaul this property? As we don't
> > >> really care backward compatibility of DTBs on those systems we can
> > >> just redesign it.
> > > You may not care about backward compatibility, but I do. We don't
> > > break existing systems, full stop.
> > Ah it won't break any existing system. Sorry for not giving enough insight
> > into the platform in previous reply. As for Loongson64 all DTBs are built
> > into kernel binary. So as long as binding are changed together with all DTS
> > in tree we won't break any system.
>
> This is factually wrong. QEMU produces a DT for Loongarch at runtime.
> So no, you're not allowed to just drop bindings on the floor. They
> stay forever.
>
> > > As for the offending property, it has no place here either. DT is not
> > > the place where you put "performance knobs".
> > Hmm, I can see various bindings with vendor prefix exposing device
> > configurations. If we seen this interrupt routing as a device configuration
> > I don't think it's against devicetree design philosophy.
>
> Just because we have tons of crap in the device trees doesn't give you
> a license to be just as bad.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.