[QUERY]: Acknowledgment of edge triggered interrupts

[Lad, Prabhakar]

Hi Marc,

On Fri, May 6, 2022 at 1:18 PM Marc Zyngier <maz at kernel.org> wrote:
>
> On Fri, 06 May 2022 12:24:34 +0100,
> "Lad, Prabhakar" <prabhakar.csengg at gmail.com> wrote:
> >
> > Hi Marc,
> >
> > Sorry for the late reply.
> >
> > On Mon, May 2, 2022 at 10:44 AM Marc Zyngier <maz at kernel.org> wrote:
> > >
> > > On Sat, 30 Apr 2022 19:41:24 +0100,
> > > "Lad, Prabhakar" <prabhakar.csengg at gmail.com> wrote:
> > > >
> > > > Hi Marc,
> > > >
> > > > I am currently working on the irq-sifive-plic.c driver. The
> > > > irq-sifive-plic.c driver is currently implemented as a chained domain.
> > > > On our SoC which uses this block for EDGE interrupts we need to first
> > > > acknowledge the interrupt before handling it.
> > >
> > > Isn't that what the CLAIM register does on the PLIC? AFAICT, this
> > > interrupt controller is able to implement the whole flow, irrespective
> > > of the trigger mechanism.
> > >
> > Yes, the CLAIM register is used to ACK interrupts.
> >
> > > The spec strongly hints at that, see [1] ("Interrupt gateways"), and
> > > the uniform handling that results of it. In a way, this is strikingly
> > > similar to what the original ARM GIC does.
> > >
> > The ARM GIC allows the next interrupts to be pending, that is it can
> > stock interrupts (pending interrupt counter).
>
> Well, if you consider a single bit a counter, yes.
>
> >
> > > [1] https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc
> > >
> > > >
> > > > I came across a similar situation on a different driver (patch [0])
> > > > but it isn't a chained handler.
> > > >
> > > > What approach should be taken for chained IRQ domains to handle such cases?
> > >
> > > I don't think there is anything to change. At least, as long as the
> > > Interrupt Gateway is doing its job correctly. How this gateway is
> > > configured is unfortunately out of the scope of the architecture, it
> > > seems, and I'd expect your HW to have some sort of knobs for the
> > > trigger type to be configured. This would be dealt with in a separate
> > > stacked driver.
> > >
> > Renesas RZ/Five Soc has a AX45MP AndesCore which has NCEPLIC100.
> > Quoting from https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#interrupt-gateways
> >
> > "If the global interrupt source was edge-triggered, the gateway will
> > convert the first matching signal edge into an interrupt request.
> > Depending on the design of the device and the interrupt handler, in
> > between sending an interrupt request and receiving notice of its
> > handler’s completion, the gateway might either ignore additional
> > matching edges or increment a counter of pending interrupts. In either
> > case, the next interrupt request will not be forwarded to the PLIC
> > core until the previous completion message has been received"
> >
> > Andes NCEPLIC100 ignores the next interrupt edge until the previous
> > completion message has been received and to top it the gateway doesn't
> > have a pending interrupt counter. So the only workaround for handling
> > edge interrupts is to first acknowledge it and then run the handler as
> > shown in the attached image.
>
> Huh. I see what you mean. The problem isn't the Ack, but the EOI. You
> need to ensure completion of the interrupt before it is handled so
> that you avoid losing bits. This is precisely what a read of CLAIM
> should have guaranteed. Who came up with this insane piece of crap?
> Really, some people shouldn't be left designing interrupt controllers.
>
Can't comment on it ;)

> I'm afraid you'll have to use a separate flow for edge interrupts,
> probably using the fasteoi_ack flow, and perform the *write* to
> COMPLETE/CLAIM in the irq_ack() callback.
>
Thanks for the pointer, I'll spin out an RFC after implementing this.

> Is this a feature of this PLIC implementation? Or is that common to
> all PLICs?
>
I think it's a feature of this PLIC implementation and not common to all PLIC's.

Cheers,
Prabhakar

>         M.
>
> --
> Without deviation from the norm, progress is not possible.