On Mon, 2008-03-17 at 17:13 +0100, Laurent Pinchart wrote: > The PIC I am working with is linked to a falling-edge external irq on the > CPM2. When the first PIC interrupt was generated the kernel called the PIC > chained irq handler endlessly. > > After some investigation it turned out the external interrupt bit in the CPM2 > interrupt pending register never got cleared. set_irq_chained_handler() > registers the chained irq handler at the lowest level in the irq stack, > bypassing all the interrupt acknowledgement/masking logic.
Yes, exactly. To answer a previous question in the thread, the reason there are two approaches to cascades is just that: - The "easy" approach using setup_irq(). The normal interrupt handling is done for the cascade, it's masked/acked/whatever-is-needed as any other interrupt before the second interrupt is fetched. This results is slightly more kernel stack usage and overhead in getting to the second interrupt, among other things, but is easier. - The "fast" approach using a chained handler. This, as you noticed, bypass pretty much the whole stack and calls the chain handler directly. That means that your chain handler is responsible to perform all the necessary things to ensure the cascade interrupt is properly ack'ed etc... > The fix was easy, all I had to do was to call desc->chip->ack(irq) at the > beginning on the chained irq handler and desc->chip->eoi(irq) at the end. For an edge cascade, that would do, I suppose. But beware that if you are only calling ack() and not mask(), then a subsequent chain interrupt from the same cascade can (and will) potentially happen while you are calling the handler as the cascade itself has been ack'ed and not masked. In the case of cpm2, that also probably means you don't need to call end(). That might be fine though, but it increases the chances of having of stack overflows caused by interrupts stacking up. > However, I'm wondering if this really belongs in the PIC code, or if PICs > shouldn't be registered at a higher level (setup_irq or even request_irq) so > that they would reuse the handle_*_irq handlers. Any opinion on this ? They can. The chain handling mechanism is an optimisation. It avoids a spinlock and other bits & pieces which improve performance & latency of handling cascaded interrupts, at the expense of that added complexity. Ben. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
