On Fri, Oct 11, 2024 at 5:40 PM Sergey Matsievskiy <matsievskiysv@xxxxxxxxx> wrote: > On Fri, Oct 11, 2024 at 11:18:55AM +0200, Linus Walleij wrote: > > I'm a bit puzzled by the patch because I don't understand it. > > The current implementation only calls chained_irq_enter() and chained_irq_exit() > if it detects pending interrupts. > > ``` > for (i = 0; i < info->stride; i++) { > uregmap_read(info->map, id_reg + 4 * i, ®); > if (!reg) > continue; > > chained_irq_enter(parent_chip, desc); > ``` > > However, in case of GPIO pin configured in level mode and the parent controller > configured in edge mode, GPIO interrupt might be lowered by the hardware. In the > result,if the interrupt is short enough, the parent interrupt is still pending > while the GPIO interrupt is cleared; chained_irq_enter() never gets called and > the system hangs trying to service the parent interrupt. > > Moving chained_irq_enter() and chained_irq_exit() outside the for loop ensures > that they are called even when GPIO interrupt is lowered by the hardware. > > The similar code with chained_irq_enter() / chained_irq_exit() functions > wrapping interrupt checking loop may be found in many other drivers: > ``` > grep -r -A 10 chained_irq_enter drivers/pinctrl > ``` > > > This needs to describe how moving the chained irq calls achieves > > this effect. > > If the explanation above satisfies you, I'll elaborate the commit message and > resend the patch. Excellent explanation Sergey, just put it all in the committ message and I'll apply it! Yours, Linus Walleij