[PATCH 3/9 v2] omap: generic: introduce a single check_revision

[PATCH 3/9 v2] omap: generic: introduce a single check_revision

[Nishanth Menon]

Introduce a single omap generic check_revision that routes the
request to the right revision of check_revision.

Cc: Tony Lindgren <tony@atomide.com>
Cc: Angelo Arrifano <miknix@gmail.com>
Cc: "Zebediah C. McClure" <zmc@lurian.net>
Cc: Alistair Buxton <a.j.buxton@gmail.com>
Cc: Grazvydas Ignotas <notasas@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
Cc: Vikram Pandita <vikram.pandita@ti.com>
Cc: Vishwanath S <vishwa.s@ti.com>
Cc: linux-omap@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
V2: comments from http://marc.info/?t=127725956100006&r=1&w=2
	fixed
V1: original
 arch/arm/mach-omap1/io.c              |    3 +--
 arch/arm/mach-omap2/io.c              |    2 +-
 arch/arm/plat-omap/common.c           |   12 ++++++++++++
 arch/arm/plat-omap/include/plat/cpu.h |   13 ++++++++++++-
 4 files changed, 26 insertions(+), 4 deletions(-)

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
+}
+
 /*
  * 32KHz clocksource ... always available, on pretty most chips except
  * OMAP 730 and 1510.  Other timers could be used as clocksources, with
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 7514174..5f12a0b 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
 
 
 int omap_chip_is(struct omap_chip_id oci);
-void omap2_check_revision(void);
+#ifdef CONFIG_ARCH_OMAP2PLUS
+extern void omap2_check_revision(void);
+#else
+static inline void omap2_check_revision(void) {}
+#endif
+
+#ifdef CONFIG_ARCH_OMAP1
+extern void omap1_check_revision(void);
+#else
+static inline void omap1_check_revision(void) {}
+#endif
+void omap_check_revision(void);
 
 /*
  * Runtime detection of OMAP3 features
-- 
1.6.3.3

[PATCH 6/9 v2] omap: move generic omap3 features to generic

[Nishanth Menon]

sgx, iva, l2cache, sgx, neon, isp are generic features, make
them generic features, current OMAP3 detection mechanism
is still retained. 192Mhz is more specific OMAP3 feature
so it is retained as is

Cc: Tony Lindgren <tony@atomide.com>
Cc: Angelo Arrifano <miknix@gmail.com>
Cc: "Zebediah C. McClure" <zmc@lurian.net>
Cc: Alistair Buxton <a.j.buxton@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
Cc: Vikram Pandita <vikram.pandita@ti.com>
Cc: Vishwanath S <vishwa.s@ti.com>
Cc: linux-omap@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
V2: rebased patch after changes to 3/9
V1: original
 arch/arm/mach-omap2/id.c              |   20 ++++++++--------
 arch/arm/plat-omap/common.c           |    4 +++
 arch/arm/plat-omap/include/plat/cpu.h |   39 +++++++++++++++++++-------------
 3 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 809e13a..01555b6 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -161,7 +161,7 @@ static void __init omap24xx_check_revision(void)
 #define OMAP3_CHECK_FEATURE(status,feat)				\
 	if (((status & OMAP3_ ##feat## _MASK) 				\
 		>> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## _NONE) { 	\
-		omap3_features |= OMAP3_HAS_ ##feat;			\
+		omap_features |= OMAP_HAS_ ##feat;			\
 	}
 
 static void __init omap3_check_features(void)
@@ -310,20 +310,20 @@ static void __init omap3_cpuinfo(void)
 		/*
 		 * AM35xx devices
 		 */
-		if (omap3_has_sgx()) {
+		if (omap_has_sgx()) {
 			omap_revision = OMAP3517_REV(rev);
 			strcpy(cpu_name, "AM3517");
 		} else {
 			/* Already set in omap3_check_revision() */
 			strcpy(cpu_name, "AM3505");
 		}
-	} else if (omap3_has_iva() && omap3_has_sgx()) {
+	} else if (omap_has_iva() && omap_has_sgx()) {
 		/* OMAP3430, OMAP3525, OMAP3515, OMAP3503 devices */
 		strcpy(cpu_name, "OMAP3430/3530");
-	} else if (omap3_has_iva()) {
+	} else if (omap_has_iva()) {
 		omap_revision = OMAP3525_REV(rev);
 		strcpy(cpu_name, "OMAP3525");
-	} else if (omap3_has_sgx()) {
+	} else if (omap_has_sgx()) {
 		omap_revision = OMAP3515_REV(rev);
 		strcpy(cpu_name, "OMAP3515");
 	} else {
@@ -354,11 +354,11 @@ static void __init omap3_cpuinfo(void)
 	/* Print verbose information */
 	pr_info("%s ES%s (", cpu_name, cpu_rev);
 
-	OMAP_SHOW_FEATURE(3, l2cache);
-	OMAP_SHOW_FEATURE(3, iva);
-	OMAP_SHOW_FEATURE(3, sgx);
-	OMAP_SHOW_FEATURE(3, neon);
-	OMAP_SHOW_FEATURE(3, isp);
+	OMAP_SHOW_FEATURE(, l2cache);
+	OMAP_SHOW_FEATURE(, iva);
+	OMAP_SHOW_FEATURE(, sgx);
+	OMAP_SHOW_FEATURE(, neon);
+	OMAP_SHOW_FEATURE(, isp);
 	OMAP_SHOW_FEATURE(3, 192mhz_clk);
 
 	printk(")\n");
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index f240d9a..60db750 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -89,6 +89,10 @@ void __init omap_reserve(void)
 	omap_vram_reserve_sdram_lmb();
 }
 
+/* OMAP Generic features */
+u32 omap_features;
+EXPORT_SYMBOL(omap_features);
+
 void __init omap_check_revision(void)
 {
 #ifdef CONFIG_ARCH_OMAP1
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index f8ecbc4..3cc4947 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -331,14 +331,14 @@ IS_OMAP_TYPE(3517, 0x3517)
 # undef cpu_is_omap3517
 # define cpu_is_omap3430()		is_omap3430()
 # define cpu_is_omap3503()		(cpu_is_omap3430() &&		\
-						(!omap3_has_iva()) &&	\
-						(!omap3_has_sgx()))
+						(!omap_has_iva()) &&	\
+						(!omap_has_sgx()))
 # define cpu_is_omap3515()		(cpu_is_omap3430() &&		\
-						(!omap3_has_iva()) &&	\
-						(omap3_has_sgx()))
+						(!omap_has_iva()) &&	\
+						(omap_has_sgx()))
 # define cpu_is_omap3525()		(cpu_is_omap3430() &&		\
-						(!omap3_has_sgx()) &&	\
-						(omap3_has_iva()))
+						(!omap_has_sgx()) &&	\
+						(omap_has_iva()))
 # define cpu_is_omap3530()		(cpu_is_omap3430())
 # define cpu_is_omap3505()		is_omap3505()
 # define cpu_is_omap3517()		is_omap3517()
@@ -457,22 +457,29 @@ static inline unsigned int omap##rev##_has_ ##feat(void)		\
 }									\
 
 /*
+ * Runtime detection of Generic OMAP features
+ */
+extern u32 omap_features;
+
+#define OMAP_HAS_L2CACHE		BIT(0)
+#define OMAP_HAS_IVA			BIT(1)
+#define OMAP_HAS_SGX			BIT(2)
+#define OMAP_HAS_NEON			BIT(3)
+#define OMAP_HAS_ISP			BIT(4)
+
+OMAP_HAS_FEATURE(, l2cache, L2CACHE)
+OMAP_HAS_FEATURE(, sgx, SGX)
+OMAP_HAS_FEATURE(, iva, IVA)
+OMAP_HAS_FEATURE(, neon, NEON)
+OMAP_HAS_FEATURE(, isp, ISP)
+
+/*
  * Runtime detection of OMAP3 features
  */
 extern u32 omap3_features;
 
-#define OMAP3_HAS_L2CACHE		BIT(0)
-#define OMAP3_HAS_IVA			BIT(1)
-#define OMAP3_HAS_SGX			BIT(2)
-#define OMAP3_HAS_NEON			BIT(3)
-#define OMAP3_HAS_ISP			BIT(4)
 #define OMAP3_HAS_192MHZ_CLK		BIT(5)
 
-OMAP_HAS_FEATURE(3, l2cache, L2CACHE)
-OMAP_HAS_FEATURE(3, sgx, SGX)
-OMAP_HAS_FEATURE(3, iva, IVA)
-OMAP_HAS_FEATURE(3, neon, NEON)
-OMAP_HAS_FEATURE(3, isp, ISP)
 OMAP_HAS_FEATURE(3, 192mhz_clk, 192MHZ_CLK)
 
 #endif
-- 
1.6.3.3

RE: [PATCH 3/9 v2] omap: generic: introduce a single check_revision

[DebBarma, Tarun Kanti]

Nishant,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] 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;
> Shilimkar, Santosh; Guruswamy, Senthilvadivu; Kevin Hilman; Tomi
> Valkeinen; Aaro Koskinen; Pandita, Vikram; S, Vishwanath; linux-
> omap@vger.kernel.org
> Subject: [PATCH 3/9 v2] omap: generic: introduce a single check_revision
> 
> Introduce a single omap generic check_revision that routes the
> request to the right revision of check_revision.
> 
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Angelo Arrifano <miknix@gmail.com>
> Cc: "Zebediah C. McClure" <zmc@lurian.net>
> Cc: Alistair Buxton <a.j.buxton@gmail.com>
> Cc: Grazvydas Ignotas <notasas@gmail.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Sanjeev Premi <premi@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
> Cc: Vikram Pandita <vikram.pandita@ti.com>
> Cc: Vishwanath S <vishwa.s@ti.com>
> Cc: linux-omap@vger.kernel.org
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> V2: comments from http://marc.info/?t=127725956100006&r=1&w=2
> 	fixed
> V1: original
>  arch/arm/mach-omap1/io.c              |    3 +--
>  arch/arm/mach-omap2/io.c              |    2 +-
>  arch/arm/plat-omap/common.c           |   12 ++++++++++++
>  arch/arm/plat-omap/include/plat/cpu.h |   13 ++++++++++++-
>  4 files changed, 26 insertions(+), 4 deletions(-)
> 
> 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");
        }
...


> +
>  /*
>   * 32KHz clocksource ... always available, on pretty most chips except
>   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-
> omap/include/plat/cpu.h
> index 7514174..5f12a0b 100644
> --- a/arch/arm/plat-omap/include/plat/cpu.h
> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
> 
> 
>  int omap_chip_is(struct omap_chip_id oci);
> -void omap2_check_revision(void);
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> +extern void omap2_check_revision(void);
> +#else
> +static inline void omap2_check_revision(void) {}
> +#endif
> +
> +#ifdef CONFIG_ARCH_OMAP1
> +extern void omap1_check_revision(void);
> +#else
> +static inline void omap1_check_revision(void) {}
> +#endif
> +void omap_check_revision(void);
> 
>  /*
>   * Runtime detection of OMAP3 features
> --
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9 v2] omap: generic: introduce a single check_revision

[Nishanth Menon]

DebBarma, Tarun Kanti had written, on 06/25/2010 08:50 AM, the following:
> Nishant,
> 
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] 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.
b)  when a new processor like omap5+ appears, yeah, we'd need to add 
cpu_is_omap5xxx() here as well..

I am biased to (a), but leave the decision to community wisdom ;)


[...]
-- 
Regards,
Nishanth Menon

Re: [PATCH 3/9 v2] omap: generic: introduce a single check_revision

[Venkatraman S]

On Fri, Jun 25, 2010 at 7:25 PM, Nishanth Menon <nm@ti.com> wrote:
> DebBarma, Tarun Kanti had written, on 06/25/2010 08:50 AM, the following:
>>
>> Nishant,
>>
>>> -----Original Message-----
>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>>> owner@vger.kernel.org] 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.

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().

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

Re: [PATCH 3/9 v2] omap: generic: introduce a single check_revision

[Nishanth Menon]

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@ti.com> wrote:
>> DebBarma, Tarun Kanti had written, on 06/25/2010 08:50 AM, the following:
>>> Nishant,
>>>
>>>> -----Original Message-----
>>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>>>> owner@vger.kernel.org] 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.

> 
> 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.
> 
> ~Venkat.


-- 
Regards,
Nishanth Menon

Re: [PATCH 3/9 v2] omap: generic: introduce a single check_revision

[Venkatraman S]

On Fri, Jun 25, 2010 at 8:13 PM, Nishanth Menon <nm@ti.com> 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@ti.com> wrote:
>>>
>>> DebBarma, Tarun Kanti had written, on 06/25/2010 08:50 AM, the following:
>>>>
>>>> Nishant,
>>>>
>>>>> -----Original Message-----
>>>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>>>>> owner@vger.kernel.org] 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9 v2] omap: generic: introduce a single check_revision

[Nishanth Menon]

S, Venkatraman had written, on 06/25/2010 10:16 AM, the following:
> On Fri, Jun 25, 2010 at 8:13 PM, Nishanth Menon <nm@ti.com> 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@ti.com> wrote:
>>>> DebBarma, Tarun Kanti had written, on 06/25/2010 08:50 AM, the following:

[...]
>>>>>> 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)

[...]

>>>> 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.

Thanks for clarifying. my bad.. missed that thread :(.

Will post a v3 - do feel free to review and reviewd/Ack if you find it ok.

> If it was indeed possible, then
> a) #ifdefs are not needed
ofcourse :)

> 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()
> 
hmm.. This will not be relevant anymore. will post a v3 which assumes 
that omap1 and omap2 are independent.
the headers ensure that null functions are introduced when the defines 
are not present.

[...]
-- 
Regards,
Nishanth Menon