Kevin Hilman wrote:
> Hemant Pedanekar <hema...@ti.com> writes:
> 
>> This patch updates the common platform files with TI816X specific
>> additions. 
>> 
>> Also adds new files for TI816X modules base addresseses and irq
>> definitions. 
>> 
[...]
>> +            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?
>

I tested this on OMAP3 but haven't verified on OMAP2. I will try to 
check this on all the variants.
  
> 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. 
>

Yes, that is better. Will add.
 
>>  9998:
>>              ldrne   \irqnr, [\base, #INTCPS_SIR_IRQ_OFFSET]
>>              and     \irqnr, \irqnr, #ACTIVEIRQ_MASK /* Clear spurious bits 
>> */
>> @@ -139,6 +148,35 @@ omap_irq_base:  .word   0
>>              .endm
>>  #endif
>> 
>> +/*
>> + * Optimized irq functions for ti816x
>> + */
>> +
>> +#ifdef CONFiG_ARCH_TI816X
>               ^
> typo?  these are case-sensitive, so this part would never be compiled
>
Yes, I will fix this.
 
[...]
> 
> Kevin


Thanks
-
Hemant

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to