Re: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Thu, Nov 29, 2007 at 03:18:04PM +0100, Remy Bohmer wrote:
> Hello Russell,
> 
> > If people insist on adding the mask/unmask crap to it, the function
> > might as well be deleted and be an alias for handle_level_IRQ.  Because
> > that's _precisely_ what you lot are turning it into.
> 
> First, I want to make clear that I am just debugging a problem on RT
> that does not exist on mainline, and I am trying to find a way to get
> it solved nicely _on RT_, and preferable in a way that it works for
> everybody with the least chance for regression.

While I realise that, I'm telling you that the _problem_ is being
caused by the wrong handler being used.

> I already mentioned that RT is doing masking in this code during
> normal use, where the mainline kernel does not do this, **except** in
> an error situation.

AT91 has edge based GPIO interrupts which need special handling so
edges aren't missed.  That's what the edge handler is there for - to
ensure that edges aren't missed while the interrupt is soft-disabled.

SA1100 and PXA have exactly the same setup.  They use the correct
handler.  Why is AT91 special?

> So, as far as I can tell , the type really used on at91 for the GPIO
> stuff _is_ a simple_irq as you describe, but one that can be
> masked/unmasked **in case of errors**. It should never be masked
> during normal use.
> 
> So, I propose option 1 to solve it on RT, and thus to trigger Steven
> to NACK my first patch. I will try and see if I can make it work
> _without_ masking on RT (except in case of errors, just as in
> mainline).
> ...and probably add some clear description about the purpose of
> simple_irq, especially related to masking...
> 
> Do you agree on this Russell?

Clearly no, I disagree.

> > Ah, and looking at the changes to the file, the addition of the mask
> > and unmask was done by someone who didn't understand what this was
> > trying to do.  So that change should be backed out.
> 
> Maybe he was trying to mask the irq during an error situation?

Who knows.  The patch says nothing about that change.  The change was
never copied to me, so I've never reviewed it (and had it been, I
would've indicated that the change was wrong.)