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

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

Jason, Ezequiel,

On 01/23/2014 12:54 PM, Ezequiel Garcia wrote:
> Sebastian,
>
> On Thu, Jan 23, 2014 at 08:10:49AM -0300, Ezequiel Garcia wrote:
>> On Thu, Jan 23, 2014 at 12:49:50AM +0100, Sebastian Hesselbarth wrote:
>> [..]
>>>> Notice that Ezequiel has added an IRQ handler that just calls panic,
>>>> so a spurious interrupt call is VERY VERY BAD.
>>>
>>> And I understand that he now clears watchdog's register before
>>> requesting an irq. All that is missing is bridge_irq driver clearing
>>> CAUSE register after masking all irqs, right?
>>>
>>
>> Are you sure clearing the CAUSE register after masking the IRQs will be enough?
>>
>> AFAICS, until now nobody unmasks the watchdog IRQ (at least the orion_wdt
>> driver didn't request the interruption) but *still* the CAUSE register is set
>> upon watchdog expiration. So I would guessed a masked interrupt still raises a
>> bit in the CAUSE register.
>>
>
> Let me add some real information instead of my speculations. Taken from
> the Kirkwood specification:
>
> Table 136: Mbus-L to Mbus Bridge Interrupt Mask Register
> Offset:		0x00020114
> Field: 		Mask
> Type/InitVal: 	RW 0x0
> Description:	There is a mask bit per each cause bit. Mask only affects the
> 		assertion of interrupt pins. It does not affect the setting of
> 		bits in the Cause register.
>
> So I guess this is why Jason has been insisting with the introduction of
> the irq_startup.
>
> (Just for reference, the little patch I attached yesterday proved to work here.)

You guys were so right ;)

I just tested your v4 of the watchdog patches and forced a stale
watchdog irq. It will cause a watchdog reset as you predicted.

I have some fixes for irq-orion.c's bridge irq that take care of:
- handle_edge_irq
- mask _and_ clear irqs on init
- use irq_enable to ensure stale irqs are acked before unmask

With the last one, the stale watchdog irq will not raise watchdog's
irq handler anymore.

BTW, during my test, it looks like RSTOUT wasn't set, so I'll add
my Tested-by on v4 when I have investigated that.

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