Re: [PATCH 13/21] arm: irq: export set flags

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 31 Mar 2011, Jonathan Cameron wrote:

> ---
>  arch/arm/kernel/irq.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
> index 3535d37..e4fd9e4 100644
> --- a/arch/arm/kernel/irq.c
> +++ b/arch/arm/kernel/irq.c
> @@ -163,6 +163,7 @@ void set_irq_flags(unsigned int irq, unsigned int iflags)
>  		desc->status &= ~IRQ_NOAUTOEN;
>  	raw_spin_unlock_irqrestore(&desc->lock, flags);
>  }
> +EXPORT_SYMBOL(set_irq_flags);

For one it would be EXPORT_SYMBOL_GPL, but that code has changed, so
the patch wont apply. Please make sure your patches apply to latest
linus git next time.

So the code looks like this now.

void set_irq_flags(unsigned int irq, unsigned int iflags)
{
	unsigned long clr = 0, set = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;

	if (irq >= nr_irqs) {
		printk(KERN_ERR "Trying to set irq flags for IRQ%d\n", irq);
		return;
	}

	if (iflags & IRQF_VALID)
		clr |= IRQ_NOREQUEST;
	if (iflags & IRQF_PROBE)
		clr |= IRQ_NOPROBE;
	if (!(iflags & IRQF_NOAUTOEN))
		clr |= IRQ_NOAUTOEN;
	/* Order is clear bits in "clr" then set bits in "set" */
	irq_modify_status(irq, clr, set & ~clr);
}

There are tons of drivers which use it under an ARM ifdef. That's
really silly. The flags are in 100% of the cases constants, so if we
remove the irq_nr check, then the compiler can optimize that code down
to
	irq_modify_status(irq, X, Y);

So we have two options here to solve that.

Use coccinelle and sweep the whole tree and convert it to

    irq_modify_status(irq, X, Y);

calls or move that set_irq_flags() stuff as an inline helper (minus
the printk) - the core code does the irq validity check already - into
linux/interrupt.h and clean up all the ifdef ARM stuff in drivers/* 

I'd vote for the first one, but I have no real objection against the
second solution.

Russell ??

Thanks,

	tglx