On Wed, 28 Aug 2019 08:27:05 +0800Hi Marc
Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote:
On 2019/8/28 äå12:45, Marc Zyngier wrote:How broken is it? You very much seem to rely on the HW being correct
On 27/08/2019 09:52, Jiaxun Yang wrote:Seems expensive but to deal with a buggy hardware... That's worthy.
+ chained_irq_enter(chip, desc);Reading the enabled status from the HW on each interrupt? I'm sure
+ pending = readl(priv->intc_base + LS3_REG_INTC_EN_STATUS) &
+ readl(priv->intc_base + LS3_REG_INTC_STATUS);
that's pretty cheap...
here, since you trust it exclusively. I'd expect the enable mask to be
a SW construct if you didn't blindly trust it
And if this is truly the right way to do it, please document theThanks, will do.
various problems with the controller so that we don't break it at a
Then how comes this comes from the irqchip's DT node? This should be
part of the endpoint's interrupt specifier.
The same.It's parent IRQ (mti,cpu-interrupt-controller) is a percpu IRQ.But then why is that interrupt described using the "core" property? It
should be an interrupt specifier, just like any other interrupt.
In design, it allows us to decide affinity at runtime but actually hardware is seriously broken.I understand the HW is terrible. But the binding looks pretty bad too.
This needs fixing.