On 01/23/2014 01:19 AM, Jason Gunthorpe wrote:
On Thu, Jan 23, 2014 at 01:03:26AM +0100, Sebastian Hesselbarth wrote:
Sebastian:
I looked at the irq-orion driver a bit more and noticed this:
ret = irq_alloc_domain_generic_chips(domain, nrirqs, 1, np->name,
handle_level_irq, clr, 0, IRQ_GC_INIT_MASK_CACHE);
^^^^^^^^^^^^^^^^^^^^^
Shouldn't it be handle_edge_irq? Otherwise who is calling irq_ack? How
does this work at all? :)
I can tell you that it comes from arch/arm/plat-orion/irq.c and I
blindly copied it. I never really checked the differences in handling
level/edge irqs. Besides, if it wasn't working, we wouldn't get far
in booting the kernel without timer irqs.
Ezequiel found the ack call I missed, so it makes sense it works.
I think the difference in routines only starts to matter when you can
get another incoming edge IRQ while already handling one (due to SMP?
threaded interrupts? RealTime? not sure)
It also remains asserted if you clear the actual cause of the interrupt
and is only asserted again on the next low-to-high transition.
Which is why Ezequiel's patch is the right approach: we need to clear
the interrupt latched in the cause register after the watchdog driver
disable but before enabling/unmasking the interrupt.
Remember, the BRIDGE_MASK register has no effect on the BRIDGE_CAUSE,
it only effects which bits propogate to the main cause register.
Yeah, I know. But you don't get new ones if mask them. At least for
edge triggered irqs, you can also clear them without clearing the
cause of the interrupt. Nevertheless, I think we agree here.
*BUT*, I will double-check how Linux deals with level/edge irqs and if
Orion SoCs have edge or level triggered cause registers. That should
reveal, if it is more sane to use handle_edge_irq here and possibly in
the main interrupt controller, too.
There is a mixture.
Any cause bit documented to be clearable is edge triggered, all others
are level.
On Kirkwood this means all of the main interrupt controller bits are
level and all the bridge bits are edge. Which means edge is
definitely correct for the bridge handler, and level correct for the
main handler.
Just checked that for Dove, it is the same there. Main IRQ_CAUSE is
RO, BRIDGE_CAUSE is RW0C, and PMU_CAUSE is RW *sigh*.
I need to remember that when Dove moves over to mach-mvebu, as we need
a different chained irq handler for PMU that deals with that broken RW
register.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
