On 2020-03-09 15:03, Linus Walleij wrote:
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'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.
--
Jazz is not dead. It just smells funny...
