On 2020-03-09 15:03, Linus Walleij wrote:
On Mon, Mar 9, 2020 at 3:54 PM Marc Zyngier maz@kernel.org 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'd rather we have the minimal change that makes your system runnable. If making irq_eoi depend on some QC magic, fine by me. Having an unbalanced enable/disable setup looks pretty fragile.
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.
I don't think anyone knows what they are outside of the usual QC circles.
M.