"Pedanekar, Hemant" <hemantp@xxxxxx> writes: > Hi Kevin, > > Kevin Hilman wrote: >> Hemant Pedanekar <hemantp@xxxxxx> writes: >> >>> This patch updates the common platform files with TI816X specific >>> additions. >>> >>> Also adds new files for TI816X modules base addresseses and irq >>> definitions. >>> >>> Signed-off-by: Hemant Pedanekar <hemantp@xxxxxx> >>> --- > [...] >>> >>> diff --git a/arch/arm/mach-omap2/include/mach/entry-macro.S >> b/arch/arm/mach-omap2/include/mach/entry-macro.S >>> index 50fd749..6516cbd 100644 >>> --- a/arch/arm/mach-omap2/include/mach/entry-macro.S >>> +++ b/arch/arm/mach-omap2/include/mach/entry-macro.S @@ -34,7 +34,7 @@ >>> .endm >>> >>> /* >>> - * Unoptimized irq functions for multi-omap2, 3 and 4 >>> + * Unoptimized irq functions for multi-omap2, 3, 4 and ti816x */ >>> >>> #ifdef MULTI_OMAP2 >>> @@ -57,7 +57,8 @@ omap_irq_base: .word 0 > [...] >>> + bne 9998f >>> + >>> + /* >>> + * ti816x has additional IRQ pending register. Checking this >>> + * register on omap2 & omap3 has no effect (read as 0). + */ >>> + ldr \irqnr, [\base, #0xf8] /* IRQ pending reg 4 */ >>> + cmp \irqnr, #0x0 >> >> This part makes me a slightly nervous. At least according to >> the TRMs, >> this address is undefined on OMAP2 & OMAP3 (yet still in the >> INTC block.) >> Was this tested on OMAP2/3 hardware and verified to return 0? >> >> You might also consider wrapping this section in >> #ifdef CONFIG_ARCH_TI816X so a multi-OMAP kernel without 816x support would >> avoid this extra read. >> > > Won't the usage of #ifdef inside MULTI_OMAP2 make things look strange? > E.g., > > #ifdef MULTI_OMAP2 > ... > #ifdef CONFIG_ARCH_TI816X > ... > #endif > #endif > > (Specifically, since there is already a custom block present in #else part?) > Yeah, I thought about that too. That's why I said "you might consider..." above. But thinking more, you're right. When we want an optimized version for a specific SoC, we just compile for that SoC and get the optimized version. Go ahead and leave out the #ifdef, but I'd still like to see some tests on OMAP2 that show that reading this undefined part of the INTC block does indeed return zero. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
