[PATCH 4/4] omap: Make get_irqnr_and_base common for mach-omap2 multiboot, v2
Tony Lindgren
tony at atomide.com
Fri Jan 22 15:39:53 EST 2010
* Russell King - ARM Linux <linux at arm.linux.org.uk> [100116 03:53]:
> On Fri, Jan 15, 2010 at 05:35:20PM -0800, Tony Lindgren wrote:
> > -#ifndef CONFIG_ARCH_OMAP4
> > +#if defined(CONFIG_ARCH_OMAP2420) || defined(CONFIG_ARCH_OMAP2430) || \
> > + defined(CONFIG_ARCH_OMAP34XX)
> > +
> > +#define INTCPS_SIR_IRQ_OFFSET 0x0040 /* Active interrupt offset */
> > +#define ACTIVEIRQ_MASK 0x7f /* Active interrupt bits */
> > +
> > +omap_irq_base: .word 0x0
>
> You have no idea where this word will be placed - it could be in the middle
> of a read only section, which might really be read only. It might also be
> out of reach of the "ldr \base, [pc, #offset]" instructions you're using
> below - which is what an "ldr \base, address" instruction really is.
>
> > +
> > .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
> > - ldr \base, =OMAP2_VA_IC_BASE
> > - ldr \irqnr, [\base, #0x98] /* IRQ pending reg 1 */
> > + ldr \base, omap_irq_base
> > + cmp \base, #0 @ is irq base configured?
> > + bne 9998f @ already configured
>
> First thing, why not use get_irqnr_preamble to load the base address?
> That means you only do all this computation once per group of IRQs
> processed.
>
> Secondly, is this really worth the overhead, or can't you move
> omap_irq_base into the data section, and have your IRQ initialization
> function set the pointer?
>
> You then don't have to make these two tests every time you check for
> an interrupt/enter to process interrupts.
Here's this patch updated. It should now be actually more optimized for
non-multi-omap configurations as it uses the get_irqnr_preamble :)
Regards,
Tony
More information about the linux-arm-kernel
mailing list