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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 01/22/2014 09:52 PM, 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.

Either you only get new edge triggered interrupts after request_irq
(sane behavior) or you might sometimes get an old pending edge
triggered interrupt after request_irq (crazy behavior).

Clearing BRIDGE_CAUSE at kernel start only shortens the racy window it
doesn't eliminate it.

Actually, I missed that we mask all BRIDGE irqs in
orion_bridge_irq_init. If we now also clear already pending irqs, that
will not raise any old interrupts as long as watchdog clears all
reasons for upstream irqs before requesting a BRIDGE irq.

In the more familiar level triggered world the driver would go to the
device and ensure it wasn't asserting an IRQ level and then do the
request_irq. This guarentees it won't get an interrupt callback.

In a edge triggered world the driver should go to the device an ensure
that it won't create a new IRQ, then do request_irq - confident that
there will NEVER be a call to the IRQ handler, under any
circumstances.

So I think edge triggered interrupts need to ack any possible old edge
trigger in the cause register before the first unmask - eg in the
setup callback.
>
So, you should also clear WDT's irq in the driver yourself to clear a
possible pending upstream BRIDGE_CAUSE.

Which isn't possible - the BRIDGE_CAUSE is owned by the irq driver and
it must be cleared there, and it must only be cleared after the wdt
has been stopped so it doesn't set it again.

I should have been more precise here: I meant watchdog driver should
clear all sources of possible upstream interrupts in its _own_
registers.

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?

I'll stich a patch for that hopefully tomorrow.

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux