Hi Marc Zyngier, Thanks for the feedback. > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge > trigger detection for TINT > > On Tue, 19 Sep 2023 18:06:54 +0100, > Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > > > > Hi Marc Zyngier, > > > > > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with > > > edge trigger detection for TINT > > > > > > On Tue, 19 Sep 2023 17:32:05 +0100, > > > Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > > > > > > [...] > > > > > > > > So you mean that you *already* lose interrupts across a disable > > > > > followed by an enable? I'm slightly puzzled... > > > > > > > > There is no interrupt lost at all. > > > > > > > > Currently this patch addresses 2 issues. > > > > > > > > Scenario 1: Extra interrupt when we select TINT source on > > > > enable_irq() > > > > > > > > Getting an extra interrupt, when client drivers calls enable_irq() > > > > during probe()/resume(). In this case, the irq handler on the > > > > Client driver just clear the interrupt status bit. > > > > > > > > Issue 2: IRQ storm when we select TINT source on enable_irq() > > > > > > > > Here as well, we are getting an extra interrupt, when client > > > > drivers calls enable_irq() during probe() and this Interrupts > > > > getting generated infinitely, when the client driver calls > > > > disable_irq() in irq handler and in in work queue calling > enable_irq(). > > > > > > How do you know this is a spurious interrupt? > > > > We have PMOD on RZ/G2L SMARC EVK. So I connected it to GPIO pin and > > other end to ground. During the boot, I get an interrupt even though > > there is no high to low transition, when the IRQ is setup in the > > probe(). From this it is a spurious interrupt. > > That doesn't really handle my question. At the point of enabling the > interrupt and consuming the edge (which is what this patch does), how do > you know you can readily discard this signal? This is a genuine question. > > Spurious interrupts at boot are common. The HW resets in a funky, > unspecified state, and it's SW's job to initialise it before letting other > agents in the system use interrupts. I got your point related to loosing interrupts. Now I can detect spurious interrupts for edge trigger. Pin controller driver has a read-only register to monitor input values of GPIO input pins, use that register values before/after rzg2l_irq_enable() with TINT Status Control Register (TSCR) in IRQ controller to detect the spurious interrupt. Eg: 1) Check PIN_43_0 value (ex: low)in pinctrl driver 2) Enable the IRQ using rzg2l_irq_enable()/ irq_chip_enable_parent()in pinctrl driver 3) Check PIN_43_0 value (ex: low) in pinctrl driver 4) Check the TINT Status Control Register(TSCR) in IRQ controller driver If the values in 1 and 3 are same and the status in 4 is set, then there is a spurious interrupt. > > > > > > For all you can tell, you are > > > just consuming an edge. I absolutely don't buy this workaround, > > > because you have no context that allows you to discriminate between > > > a real spurious interrupt and a normal interrupt that lands while > > > the interrupt line was masked. > > > > > > > Currently we are not loosing interrupts, but we are getting > > > > additional > > > > Interrupt(phantom) which is causing the issue. > > > > > > If you get an interrupt at probe time in the endpoint driver, that's > > > probably because the device is not in a quiescent state when the > > > interrupt is requested. And it is probably this that needs addressing. > > > > Any pointer for addressing this issue? > > Nothing but the most basic stuff: you should make sure that the interrupt > isn't enabled before you can actually handle it, and triage it as spurious. For the GPIO interrupt case I have, RTC driver(endpoint)--> Pin controller driver -->IRQ controller driver-->GIC controller. 1) I have configured the pin as GPIO interrupts in pin controller driver 2) Set the IRQ detection in IRQ controller for edge trigger 3) The moment I set the IRQ source in IRQ controller I get an interrupt, even though there is no voltage transition. Here the system is setup properly, but there is a spurious interrupt. Currently don't know how to handle it? Any pointers for handling this issue? Note: Currently the pin controller driver is not configuring GPIO as GPIO input in Port Mode Register for the GPIO interrupts instead it is using reset value which is "Hi-Z". I will send a patch to fix it. Cheers, Biju