Re: [PATCH] of/irq: Add a quirk for controllers with their own definition of interrupt-map

[Rob Herring]

On Mon, Nov 29, 2021 at 6:13 AM Lad, Prabhakar
<prabhakar.csengg@xxxxxxxxx> wrote:
>

I timed my vacation well...

> On Mon, Nov 29, 2021 at 10:34 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >
> > On Sat, 27 Nov 2021 00:42:49 +0000,
> > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
> > >
> > > Hi Marc,
> > >
> > > > -----Original Message-----
> > > > From: Marc Zyngier <maz@xxxxxxxxxx>
> > > > Sent: 23 November 2021 09:11
> > > > To: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; kernel-team@xxxxxxxxxxx; Rob Herring
> > > > <robh@xxxxxxxxxx>; John Crispin <john@xxxxxxxxxxx>; Biwen Li <biwen.li@xxxxxxx>; Chris Brandt
> > > > <Chris.Brandt@xxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx; Prabhakar Mahadev Lad
> > > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > > Subject: Re: [PATCH] of/irq: Add a quirk for controllers with their own definition of interrupt-map
> > > >
> > > > On Tue, 23 Nov 2021 08:44:19 +0000,
> > > > Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > >
> > > > > Hi Marc,
> > > > >
> > > > > On Tue, Nov 23, 2021 at 9:33 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > > > > > On Tue, 23 Nov 2021 07:57:48 +0000,
> > > > > > Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > > > > Summarized:
> > > > > > >   - Before the bad commit, and after your fix, irqc-rza1 is invoked,
> > > > > > >     and the number of interrupts seen is correct, but input events
> > > > > > >     are doubled.
> > > > > > >   - After the bad commit, irqc-rza1 is not invoked, and there is an
> > > > > > >     interrupt storm, but input events are OK.
> > > > > >
> > > > > > OK, that's reassuring, even if the "twice the events" stuff isn't
> > > > > > what you'd expect. We at least know this is a separate issue, and
> > > > > > that this patch on top of -rc1 brings you back to the 5.15 behaviour.
> > > > > >
> > > > > > I'd expect it to be the case for the other platforms as well.
> > > > >
> > > > > OK.
> > > > >
> > > > > BTW, what would have been the correct way to do this for irqc-rza1?
> > > > > I think we're about to make the same mistake with RZ/G2L IRQC
> > > > > support[1]?
> > > >
> > > > Indeed, and I was about to look into it.
> > > >
> > > > There are multiple ways to skin this cat, including renaming 'interrupt-map' to 'my-own-private-
> > > > interrupt-map'. Or use something akin the new 'msi-range' (which we could call interrupt-range), and
> > > > replace:
> > > >
> > > >   interrupt-map = <0 0 &gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <1 0 &gic GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <2 0 &gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <3 0 &gic GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <4 0 &gic GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <5 0 &gic GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <6 0 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
> > > >                   <7 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> > > >
> > > > with:
> > > >
> > > >   interrupt-range = <&gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH 0 8>;

interrupts would work just fine here:

interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
  <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;

We don't need a different solution for N:1 interrupts from N:M. Sure,
that could become unweldy if there are a lot of interrupts (just like
interrupt-map), but is that an immediate problem?

> > > >
> > > Just to clarify, do you suggest to add interrupt-range as a generic
> > > DT property or SoC/company specific property?
> >
> > As a generic one. I have no interest in SoC-specific stuff (though you
> > are free to invent your own and run it by Rob).
> >
> OK will go with a generic one.
>
> > > If you meant to add generic property where would you suggest to
> > > document this property?
> >
> > Ideally collocated with the rest of the interrupt properties.
> >
> OK, I will go with interrupts.txt for now. Is that OK with you Rob?

Nope. As the rest of interrupts are documented in DT Spec it goes
there and there needs to be a schema (in dtschema).

> (the reason I ask because interrupt-map/mask haven't been documented).

Yes, they are. Only for the last 20+ years.

Rob