genirq default_disable()

genirq default_disable()

[Pawel MOLL]

Folks,

A quick question about the interrupt management... (the story takes
place in kernel/irq/chip.c ;-)

Here we have the default_enable():

static void default_enable(unsigned int irq)
{
        struct irq_desc *desc = irq_to_desc(irq);

        desc->chip->unmask(irq);
        desc->status &= ~IRQ_MASKED;
}

It calls chip->unmask(), which absolutely makes sense...

The default_disable(), however, is not symmetric:

static void default_disable(unsigned int irq)
{
}

Is there any reason why it shouldn't call chip->mask()?

I'll be more then happy to prepare a patch doing so, but maybe it's a
feature not a bug and I'm just missing something?

Best regards

Paweł

Re: genirq default_disable()

[Thomas Gleixner]

On Tue, 23 Jun 2009, Pawel MOLL wrote:
> Folks,
> 
> A quick question about the interrupt management... (the story takes
> place in kernel/irq/chip.c ;-)
> 
> Here we have the default_enable():
> 
> static void default_enable(unsigned int irq)
> {
>         struct irq_desc *desc = irq_to_desc(irq);
> 
>         desc->chip->unmask(irq);
>         desc->status &= ~IRQ_MASKED;
> }
> 
> It calls chip->unmask(), which absolutely makes sense...
> 
> The default_disable(), however, is not symmetric:
> 
> static void default_disable(unsigned int irq)
> {
> }
> 
> Is there any reason why it shouldn't call chip->mask()?
> 
> I'll be more then happy to prepare a patch doing so, but maybe it's a
> feature not a bug and I'm just missing something?

Yup, it's a feature: delayed irq disable.

Disabling irqs on the hardware level can be expensive. So we keep them
enabled and mark the irq as disabled. When an interrupt comes in
during that time then we disable the irq on the hardware level for
real and note internaly that it happened. On enable we replay the
interrupt so nothing is lost. That replay can be in hardware or in
software.

Thanks,

	tglx