lkml.org 
[lkml]   [2008]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [BUG][RFC] [GENERIC IRQ] irq_chip_set_defaults shutdown / disable
    On Mon, 18 Feb 2008, Hennerich, Michael wrote:
    > RESENT: Add Maintainers
    >
    > free_irq() does not disable/mask the irq, in case disable or shutdown in struct irq_chip is left uninitilazied.
    >
    > /**
    > * struct irq_chip - hardware interrupt chip descriptor
    > *
    > * @name: name for /proc/interrupts
    > * @startup: start up the interrupt (defaults to ->enable if NULL)
    > * @shutdown: shut down the interrupt (defaults to ->disable if NULL)
    > * @enable: enable the interrupt (defaults to chip->unmask if NULL)
    > * @disable: disable the interrupt (defaults to chip->mask if NULL)
    >
    >
    > According to linux/irq.h struct irq_chip information,
    > chip->disable should default to chip->mask if NULL.
    > However irq_chip_set_defaults(struct irq_chip *chip) will set it to default_disable an empty function.
    >
    > In earlier kernel versions such as 2.6.19 default_disable called chip->mask.
    > In 2.6.22 and 2.6.24 default_disable is an empty function.
    >
    > Looking through various architectures, it's still pretty common that disable and shutdown is NULL.

    Yeah, you are right. This was broken by commit
    76d2160147f43f982dfe881404cfde9fd0a9da21

    > Do I miss something here?

    Just a small detail. The commit optimized the irq_disable/enable logic
    by defaulting to delayed disable. So we want to keep that logic intact
    in case that we still have a device which is handling this
    interrupt. For the free_irq() case your patch is correct.

    The patch below fixes the shutdown case and keeps the delayed disable
    logic intact.

    How did you notice ? I guess you got spurious interrupts after calling
    free_irq(), right ?

    Thanks for analyzing and reporting,

    tglx

    ------------->
    Subject: genirq: do not leave interupts enabled on free_irq
    From: Thomas Gleixner <tglx@linutronix.de>
    Date: Mon, 18 Feb 2008 18:25:17 +0100

    The default_disable() function was changed in commit:

    76d2160147f43f982dfe881404cfde9fd0a9da21
    genirq: do not mask interrupts by default

    It removed the mask function in favour of the default delayed
    interrupt disable handling. Unfortunately this also broke the shutdown
    in free_irq() when the last handler is removed from the interrupt for
    those architectures which rely on the default implementations. Now we
    can end up with a enabled interrupt line after the last handler was
    removed.

    Fix this by adding a default_shutdown function, which is only
    installed, when the irqchip implementation does provide neither a
    shutdown nor a disable function.

    Pointed-out-by: Michael Hennerich <Michael.Hennerich@analog.com>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    CC: stable@kernel.org

    ---
    kernel/irq/chip.c | 14 +++++++++++++-
    1 file changed, 13 insertions(+), 1 deletion(-)

    Index: linux-2.6/kernel/irq/chip.c
    ===================================================================
    --- linux-2.6.orig/kernel/irq/chip.c
    +++ linux-2.6/kernel/irq/chip.c
    @@ -246,6 +246,17 @@ static unsigned int default_startup(unsi
    }

    /*
    + * default shutdown function
    + */
    +static void default_shutdown(unsigned int irq)
    +{
    + struct irq_desc *desc = irq_desc + irq;
    +
    + desc->chip->mask(irq);
    + desc->status &= ~IRQ_MASKED;
    +}
    +
    +/*
    * Fixup enable/disable function pointers
    */
    void irq_chip_set_defaults(struct irq_chip *chip)
    @@ -257,7 +268,8 @@ void irq_chip_set_defaults(struct irq_ch
    if (!chip->startup)
    chip->startup = default_startup;
    if (!chip->shutdown)
    - chip->shutdown = chip->disable;
    + chip->shutdown = chip->disable != default_disable ?
    + chip_disable : default_shutdown;
    if (!chip->name)
    chip->name = chip->typename;
    if (!chip->end)

    \
     
     \ /
      Last update: 2008-02-18 18:53    [W:3.288 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site