On 2021-01-04 03:46, Samuel Holland wrote:
On 1/3/21 7:10 AM, Marc Zyngier wrote:
On Sun, 03 Jan 2021 12:08:43 +0000,
Samuel Holland <samuel@xxxxxxxxxxxx> wrote:
On 1/3/21 5:27 AM, Marc Zyngier wrote:
[...]
For edge interrupts, don't you want to ack as early as possible,
before the handler clears the source of the interrupt? That way if a
second interrupt comes in while you're handling the first one, you
don't ack the second one without handling it?
It completely depends on what this block does. If, as I expect, it
latches the interrupt, then it needs clearing after the GIC has acked
the incoming interrupt.
Yes, there is an internal S/R latch.
- For edge interrupts, the latch is set once for each pulse.
- For level interrupts, it gets set continuously as long as the
pin is high/low.
- Writing a "1" to bit 0 of PENDING resets the latch.
- The output of the latch goes to the GIC.
It also begs the question: why would you want to clear the signal to
the GIC on mask (or unmask)? The expectations are that a pending
interrupt is preserved across a mask/unmask sequence.
I hadn't thought about anything masking the IRQ outside of the
handler; but you're right, this breaks that case. I'm trying to work
within the constraints of stacking the GIC driver, which assumes
handle_fasteoi_irq, so it sounds like I should switch back to
handle_fasteoi_ack_irq and use .irq_ack. Or based on your previous
paragraph, maybe I'm missing some other consideration?
handle_fasteoi_ack_irq() sounds like a good match for edge
interrupts. Do you actually need to do anything for level signals? If
you do, piggybacking on .irq_eoi would do the trick.
For level interrupts, I have to reset the latch (see above) after the
source of
the interrupt is cleared.
Right, so that is definitely to be done in .irq_eoi, at least in the
non-threaded case (as it doesn't involve masking/unmasking).
That was the bug with v2: I set IRQ_EOI_THREADED so .irq_eoi would run
after the
thread. But with GICv2 EOImode==0, that blocked other interrupts from
being
received during the IRQ thread. Which is why I moved it to .irq_unmask
and
removed the flag: so .irq_eoi runs at the end of the hardirq
(unblocking further
interrupts at the GIC), and .irq_unmask resets the latch at the end of
the thread.
With the flag removed, but still clearing the latch in .irq_eoi, every
edge IRQ
edge? Didn't you mean level here? Edge interrupts really should clear
the latch in .irq_ack.
was followed by a second, spurious IRQ after the thread finished.
Does that make sense?
It does. It is a bit of a kludge, but hey, silly HW (if only this could
be
turned into a bypass, it'd all be simpler).
To sum it up, this is what I'd expect to see:
For edge interrupts:
- clear latch in .irq_ack and .irq_set_irqchip_state(PENDING)
- interrupt flow set to fasteoi_ack
For level interrupts
- clear latch in .irq_eoi (non-threaded) and .irq_unmask (threaded)
- interrupt flow set to fasteoi (though leaving to the _ack version
should not hurt).
Thanks,
M.
--
Jazz is not dead. It just smells funny...
