On Wed, 2007-11-28 at 15:38 +0100, Remy Bohmer wrote:
> Hello Daniel,
>
> > * Note: The caller is expected to handle the ack, clear, mask and
> > * unmask issues if necessary.
> > So we shouldn't need any flow control unless there is some other
> > factors..
>
> This comment can be misinterpreted, I think. Who is assumed to be the
> caller in this context? The 2 other routines in the driver that
> actually do the unmasking stuff besides only calling this routine? Is
> it allowed to call it directly or should it always be done through a
> wrapper that does all these special things?
The later I think ..
> Either way, only masking interrupts, and never unmasking it, is a bug.
> If interrupts come and go slow enough you never run into this problem,
> and if this type is not used often, nobody will notice it.
> Usually interrupts needs clearence of the source before the hardware
> can generate a new one. GPIO interrupts are different, they are
> generated whenever a IO-level changes, there is no acknowledge or
> clearing of the interupt needed. These types of interrupts are never
> 'pending' from hardware point of view. So, with these type of
> interrupts, a new one can occur while the interrupt handler has not
> handled the previous one yet, and therefor these interrupt-types will
> show this bug.
Yeah, it's clear there needs to be an unmask for this special case..
I've attached a patch which only handles the special case.. Could you
test/review it..
> >
> > Additionally, we have a patch in the real time tree called
> > "irq-mask-fix.patch" which adds an "unmask" to handle_simple_irq, but as
> > the note says we don't need flow control..
>
> You mean the Montavista real time tree?
No .. I wouldn't comment about an company specific tree. I was talking
about the broken out real time patches.
Daniel
--------
Remove the IRQ_PENDING flag if it's asserted, and unmask the irq. Also loop
around to account for the pending interrupt.
Signed-Off-By: Daniel Walker <dwalker@xxxxxxxxxx>
---
kernel/irq/manage.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
Index: linux-2.6.23/kernel/irq/manage.c
===================================================================
--- linux-2.6.23.orig/kernel/irq/manage.c
+++ linux-2.6.23/kernel/irq/manage.c
@@ -646,7 +646,7 @@ __setup("hardirq-preempt=", hardirq_pree
/*
* threaded simple handler
*/
-static void thread_simple_irq(irq_desc_t *desc)
+static void thread_core_irq(irq_desc_t *desc)
{
struct irqaction *action = desc->action;
unsigned int irq = desc - irq_desc;
@@ -664,13 +664,35 @@ static void thread_simple_irq(irq_desc_t
}
/*
+ * threaded fasteoi type irq handler
+ */
+static void thread_simple_irq(irq_desc_t *desc)
+{
+ unsigned int irq = desc - irq_desc;
+
+ do {
+ /*
+ * When another irq arrived while we were handling
+ * one, we could have masked the irq.
+ * Renable it, if it was not disabled in meantime.
+ */
+ if (unlikely(desc->status & IRQ_PENDING)) {
+ desc->status &= ~IRQ_PENDING;
+ desc->chip->unmask(irq);
+ }
+ thread_core_irq(desc);
+ } while ((desc->status & (IRQ_PENDING | IRQ_INPROGRESS)));
+
+}
+
+/*
* threaded level type irq handler
*/
static void thread_level_irq(irq_desc_t *desc)
{
unsigned int irq = desc - irq_desc;
- thread_simple_irq(desc);
+ thread_core_irq(desc);
if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
desc->chip->unmask(irq);
}
@@ -682,7 +704,7 @@ static void thread_fasteoi_irq(irq_desc_
{
unsigned int irq = desc - irq_desc;
- thread_simple_irq(desc);
+ thread_core_irq(desc);
if (!(desc->status & IRQ_DISABLED) && desc->chip->unmask)
desc->chip->unmask(irq);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
