Re: [PATCH v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear

[Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 01/22/2014 11:31 PM, Jason Gunthorpe wrote:
> On Wed, Jan 22, 2014 at 07:12:38PM -0300, Ezequiel Garcia wrote:
>> On Wed, Jan 22, 2014 at 01:52:13PM -0700, Jason Gunthorpe wrote:
>>>> Clearing BRIDGE_CAUSE will only clear all currently pending upstream
>>>> IRQs, of course. If WDT IRQ will be re-raised right after that in
>>>> BRIDGE_CAUSE depends on the actual HW implementation, i.e. we do no
>>>> clear the causing IRQ itself but just what it raised in BRIDGE_CAUSE.
>>>
>>> Which is why it makes no sense to clear it one time at kernel start.
>>>
>>
>> So, it seems we need to handle irq_startup(), as you suggested.
>> I've just tested the attached patch, and it's working fine: the driver's
>> probe() fully stops the watchdog, and then request_irq() acks and
>> pending interrupts, through the added irq_startup().
>>
>> How does it look?
>
> Looks sane to me.
>
> I looked some more and there are other drivers (eg irq-metag-ext) that
> take this same approach.
>
> 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.

 From a HW point-of-view, I'd say the difference is that an
edge-triggered irq samples level transitions from e.g. low to high.
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.

A level-triggered irq will be asserted as long as the cause of the irq
is asserted. If you clear the cause, you'll also clear that bit in the
upstream controller.

*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.

Sebastian


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html