On Fri, Jun 25, 2010 at 8:13 PM, Nishanth Menon <nm@xxxxxx> wrote: > S, Venkatraman had written, on 06/25/2010 09:38 AM, the following: >> >> On Fri, Jun 25, 2010 at 7:25 PM, Nishanth Menon <nm@xxxxxx> wrote: >>> >>> DebBarma, Tarun Kanti had written, on 06/25/2010 08:50 AM, the following: >>>> >>>> Nishant, >>>> >>>>> -----Original Message----- >>>>> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- >>>>> owner@xxxxxxxxxxxxxxx] On Behalf Of Menon, Nishanth >>>>> Sent: Friday, June 25, 2010 6:57 PM >>>>> To: linux-omap >>>>> Cc: Menon, Nishanth; Tony Lindgren; Angelo Arrifano; Zebediah C. >>>>> McClure; >>>>> Alistair Buxton; Grazvydas Ignotas; Paul Walmsley; Premi, Sanjeev; >>> >>> [...] >>> >>>>> diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c >>>>> index e4d8680..4f9ee73 100644 >>>>> --- a/arch/arm/mach-omap1/io.c >>>>> +++ b/arch/arm/mach-omap1/io.c >>>>> @@ -20,7 +20,6 @@ >>>>> >>>>> #include "clock.h" >>>>> >>>>> -extern void omap1_check_revision(void); >>>>> extern void omap_sram_init(void); >>>>> >>>>> /* >>>>> @@ -102,7 +101,7 @@ void __init omap1_map_common_io(void) >>>>> /* We want to check CPU revision early for cpu_is_omapxxxx() >>>>> macros. >>>>> * IO space mapping must be initialized before we can do that. >>>>> */ >>>>> - omap1_check_revision(); >>>>> + omap_check_revision(); >>>>> >>>>> #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850) >>>>> if (cpu_is_omap7xx()) { >>>>> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c >>>>> index 4e1f53d..eeb0e30 100644 >>>>> --- a/arch/arm/mach-omap2/io.c >>>>> +++ b/arch/arm/mach-omap2/io.c >>>>> @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void) >>>>> local_flush_tlb_all(); >>>>> flush_cache_all(); >>>>> >>>>> - omap2_check_revision(); >>>>> + omap_check_revision(); >>>>> omap_sram_init(); >>>>> } >>>>> >>>>> diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c >>>>> index fca73cd..f240d9a 100644 >>>>> --- a/arch/arm/plat-omap/common.c >>>>> +++ b/arch/arm/plat-omap/common.c >>>>> @@ -89,6 +89,18 @@ void __init omap_reserve(void) >>>>> omap_vram_reserve_sdram_lmb(); >>>>> } >>>>> >>>>> +void __init omap_check_revision(void) >>>>> +{ >>>>> +#ifdef CONFIG_ARCH_OMAP1 >>>>> + if (cpu_is_omap7xx() || cpu_is_omap15xx() || cpu_is_omap16xx()) >>>>> + omap1_check_revision(); >>>>> +#endif >>>>> +#ifdef CONFIG_ARCH_OMAP2PLUS >>>>> + if (cpu_is_omap24xx() || cpu_is_omap34xx() || >>>>> cpu_is_omap44xx()) >>>>> + omap2_check_revision(); >>>>> +#endif >>>>> +} >>>> >>>> Inside omap2_check_revision() there is already check for cpu type. So do >>>> we need to have it here? Here is the code snippet!! >>>> >>>> void __init omap2_check_revision(void) >>>> { >>>> /* >>>> * At this point we have an idea about the processor revision set >>>> * earlier with omap2_set_globals_tap(). >>>> */ >>>> if (cpu_is_omap24xx()) { >>>> omap24xx_check_revision(); >>>> } else if (cpu_is_omap34xx()) { >>>> omap3_check_revision(); >>>> omap3_check_features(); >>>> omap3_cpuinfo(); >>>> return; >>>> } else if (cpu_is_omap44xx()) { >>>> omap4_check_revision(); >>>> return; >>>> } else { >>>> pr_err("OMAP revision unknown, please fix!\n"); >>>> } >>>> ... >>> >>> thanks for your comment. >>> >>> My rationale for doing it is to allow for a single OMAP build for both >>> omap1 >>> and omap2+ in which case the cpu_is check makes sense. >>> we have two choices: >>> a) remove the hope of having a single omap build (and the above logic is >>> a >>> bit simpler. >> >> I think Tarun Kanti intended to point out the redundancy within the >> OMAP2PLUS build path. > > yes I am aware of that. but consider the following: > CONFIG_ARCH_OMAP1 and CONFIG_ARCH_OMAP2PLUS being defined at the same time. > > the logic will enter without a reason for it to do so, instead it will print > OMAP revision unknown for OMAP1 - not desired. AFAIK, Tony has ruled out OMAP1 _and_ OMAP2+ multi-omap build. If it was indeed possible, then a) #ifdefs are not needed b) omap2_check_revision() shouldn't emit the warning, as it doesn't cater to all SoCs. omap99_check_revision() could be in the later code path of omap_check_revision() > >> >> i.e the cpu checks >>>>> >>>>> +#ifdef CONFIG_ARCH_OMAP2PLUS >>>>> + if (cpu_is_omap24xx() || cpu_is_omap34xx() || >>>>> cpu_is_omap44xx()) >> >> ^^^ are not needed, as the omap2_check_revision does it anyway. >> >> Then eventually omap_is_55xx() would be needed only inside >> omap2_check_revision, and not in >> omap_check_revision(). > > yes if we dont depend of if check as I mentioned above, I agree. > > option a: as above > option b: > +#ifdef CONFIG_ARCH_OMAP1 > + omap1_check_revision(); > +#endif > +#ifdef CONFIG_ARCH_OMAP2PLUS > + omap2_check_revision(); > +#endif > > I know this is what Tarun and you are proposing, i dont have a strong > feeling against it if community is aligned on it. >> -- 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
