lkml.org 
[lkml]   [2007]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH PREEMPT_RT]: On AT91 ARM: GPIO Interrupt handling can/will stall forever
    From
    Date
    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@mvista.com>

    ---
    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-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-11-28 16:47    [W:4.217 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site