On Mon, Mar 9, 2020 at 3:54 PM Marc Zyngier <maz@xxxxxxxxxx> wrote: > On 2020-03-09 12:52, Linus Walleij wrote: > > ChangeLog v1->v2: > > - Noticed that the previous solution doesn't actually work, > > the machine hangs and reboots intead (even if it got rid of > > the most obvious crash). Make a more thorough solution that > > completely avoids using these callbacks if we don't have > > a parent. > > What is the problem with disable exactly? There is no problem with .irq_disable, the system still works if I keep that. But since the original patch added these two callbacks for hierarchical I just moved them both to be conditional. The .irq_eoi callback is the culprit. > > pctrl->irq_chip.name = "msmgpio"; > > pctrl->irq_chip.irq_enable = msm_gpio_irq_enable; > > - pctrl->irq_chip.irq_disable = msm_gpio_irq_disable; > > I find it really odd to have the enable callback, but not the disable. > What is the rational for that? Can we drop the enable as well for old > platforms and only use mask/unmask instead? Hm I'm just working with the regression, and before the patch I'm fixing the driver actually had just the .irq_enable callback, so I'm restoring that state. Would you prefer a patch where I just move the assignment of the .irq_eoi callback to be conditional? I have no idea *why* .irq_eoi() locks up the system, I suspect one of those irqchip internal semantics that are sometimes not entirely clear. Yours, Linus Walleij
