[PATCH-V3 0/2] arm:omap:cleanup & split omap2/3/4_check_revision function

[PATCH-V3 0/2] arm:omap:cleanup & split omap2/3/4_check_revision function

[Vaibhav Hiremath]

Change log:
==========
Changes from V2:
	- Split the patch to make it more readable.
	- Make omap3_cpuinfo void funtion, since cpu_rev is global now.

Changes from V1:
	- Incorporated suggessions from Tony to split the function

Vaibhav Hiremath (2):
  arm:omap: Make cpu_rev static global variable
  arm:omap: cleanup & split omap2/3/4_check_revision function

 arch/arm/mach-omap2/id.c              |  186 ++++++++++++++-------------------
 arch/arm/mach-omap2/io.c              |    9 ++-
 arch/arm/plat-omap/include/plat/cpu.h |    7 +-
 3 files changed, 92 insertions(+), 110 deletions(-)

[PATCH-V3 1/2] arm:omap: Make cpu_rev static global variable

[Vaibhav Hiremath]

As part of omap revision code cleanup, make cpu_rev variable static
global to the file (id.c).

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
CC: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/id.c |  146 +++++++++++++++++++++++-----------------------
 1 files changed, 72 insertions(+), 74 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 6c58266..752dcae 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -29,7 +29,7 @@
 #include "control.h"

 static unsigned int omap_revision;
-
+static const char *cpu_rev;
 u32 omap_features;

 unsigned int omap_rev(void)
@@ -166,6 +166,56 @@ static void __init omap24xx_check_revision(void)
 	pr_info("\n");
 }

+#define OMAP3_SHOW_FEATURE(feat)		\
+	if (omap3_has_ ##feat())		\
+		printk(#feat" ");
+
+static void __init omap3_cpuinfo(void)
+{
+	const char *cpu_name;
+
+	/*
+	 * OMAP3430 and OMAP3530 are assumed to be same.
+	 *
+	 * OMAP3525, OMAP3515 and OMAP3503 can be detected only based
+	 * on available features. Upon detection, update the CPU id
+	 * and CPU class bits.
+	 */
+	if (cpu_is_omap3630()) {
+		cpu_name = "OMAP3630";
+	} else if (cpu_is_omap3517()) {
+		/* AM35xx devices */
+		cpu_name = (omap3_has_sgx()) ? "AM3517" : "AM3505";
+	} else if (cpu_is_ti816x()) {
+		cpu_name = "TI816X";
+	} else if (cpu_is_am335x()) {
+		cpu_name =  "AM335X";
+	} else if (cpu_is_ti814x()) {
+		cpu_name = "TI814X";
+	} else if (omap3_has_iva() && omap3_has_sgx()) {
+		/* OMAP3430, OMAP3525, OMAP3515, OMAP3503 devices */
+		cpu_name = "OMAP3430/3530";
+	} else if (omap3_has_iva()) {
+		cpu_name = "OMAP3525";
+	} else if (omap3_has_sgx()) {
+		cpu_name = "OMAP3515";
+	} else {
+		cpu_name = "OMAP3503";
+	}
+
+	/* Print verbose information */
+	pr_info("%s ES%s (", cpu_name, cpu_rev);
+
+	OMAP3_SHOW_FEATURE(l2cache);
+	OMAP3_SHOW_FEATURE(iva);
+	OMAP3_SHOW_FEATURE(sgx);
+	OMAP3_SHOW_FEATURE(neon);
+	OMAP3_SHOW_FEATURE(isp);
+	OMAP3_SHOW_FEATURE(192mhz_clk);
+
+	printk(")\n");
+}
+
 #define OMAP3_CHECK_FEATURE(status,feat)				\
 	if (((status & OMAP3_ ##feat## _MASK) 				\
 		>> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## _NONE) { 	\
@@ -231,7 +281,7 @@ static void __init ti81xx_check_features(void)
 	omap_features = OMAP3_HAS_NEON;
 }

-static void __init omap3_check_revision(const char **cpu_rev)
+static void __init omap3_check_revision(void)
 {
 	u32 cpuid, idcode;
 	u16 hawkeye;
@@ -245,7 +295,7 @@ static void __init omap3_check_revision(const char **cpu_rev)
 	cpuid = read_cpuid(CPUID_ID);
 	if ((((cpuid >> 4) & 0xfff) == 0xc08) && ((cpuid & 0xf) == 0x0)) {
 		omap_revision = OMAP3430_REV_ES1_0;
-		*cpu_rev = "1.0";
+		cpu_rev = "1.0";
 		return;
 	}

@@ -266,26 +316,26 @@ static void __init omap3_check_revision(const char **cpu_rev)
 		case 0: /* Take care of early samples */
 		case 1:
 			omap_revision = OMAP3430_REV_ES2_0;
-			*cpu_rev = "2.0";
+			cpu_rev = "2.0";
 			break;
 		case 2:
 			omap_revision = OMAP3430_REV_ES2_1;
-			*cpu_rev = "2.1";
+			cpu_rev = "2.1";
 			break;
 		case 3:
 			omap_revision = OMAP3430_REV_ES3_0;
-			*cpu_rev = "3.0";
+			cpu_rev = "3.0";
 			break;
 		case 4:
 			omap_revision = OMAP3430_REV_ES3_1;
-			*cpu_rev = "3.1";
+			cpu_rev = "3.1";
 			break;
 		case 7:
 		/* FALLTHROUGH */
 		default:
 			/* Use the latest known revision as default */
 			omap_revision = OMAP3430_REV_ES3_1_2;
-			*cpu_rev = "3.1.2";
+			cpu_rev = "3.1.2";
 		}
 		break;
 	case 0xb868:
@@ -298,13 +348,13 @@ static void __init omap3_check_revision(const char **cpu_rev)
 		switch (rev) {
 		case 0:
 			omap_revision = OMAP3517_REV_ES1_0;
-			*cpu_rev = "1.0";
+			cpu_rev = "1.0";
 			break;
 		case 1:
 		/* FALLTHROUGH */
 		default:
 			omap_revision = OMAP3517_REV_ES1_1;
-			*cpu_rev = "1.1";
+			cpu_rev = "1.1";
 		}
 		break;
 	case 0xb891:
@@ -313,60 +363,60 @@ static void __init omap3_check_revision(const char **cpu_rev)
 		switch(rev) {
 		case 0: /* Take care of early samples */
 			omap_revision = OMAP3630_REV_ES1_0;
-			*cpu_rev = "1.0";
+			cpu_rev = "1.0";
 			break;
 		case 1:
 			omap_revision = OMAP3630_REV_ES1_1;
-			*cpu_rev = "1.1";
+			cpu_rev = "1.1";
 			break;
 		case 2:
 		/* FALLTHROUGH */
 		default:
 			omap_revision = OMAP3630_REV_ES1_2;
-			*cpu_rev = "1.2";
+			cpu_rev = "1.2";
 		}
 		break;
 	case 0xb81e:
 		switch (rev) {
 		case 0:
 			omap_revision = TI8168_REV_ES1_0;
-			*cpu_rev = "1.0";
+			cpu_rev = "1.0";
 			break;
 		case 1:
 		/* FALLTHROUGH */
 		default:
 			omap_revision = TI8168_REV_ES1_1;
-			*cpu_rev = "1.1";
+			cpu_rev = "1.1";
 			break;
 		}
 		break;
 	case 0xb944:
 		omap_revision = AM335X_REV_ES1_0;
-		*cpu_rev = "1.0";
+		cpu_rev = "1.0";
 	case 0xb8f2:
 		switch (rev) {
 		case 0:
 		/* FALLTHROUGH */
 		case 1:
 			omap_revision = TI8148_REV_ES1_0;
-			*cpu_rev = "1.0";
+			cpu_rev = "1.0";
 			break;
 		case 2:
 			omap_revision = TI8148_REV_ES2_0;
-			*cpu_rev = "2.0";
+			cpu_rev = "2.0";
 			break;
 		case 3:
 		/* FALLTHROUGH */
 		default:
 			omap_revision = TI8148_REV_ES2_1;
-			*cpu_rev = "2.1";
+			cpu_rev = "2.1";
 			break;
 		}
 		break;
 	default:
 		/* Unknown default to latest silicon rev as default */
 		omap_revision = OMAP3630_REV_ES1_2;
-		*cpu_rev = "1.2";
+		cpu_rev = "1.2";
 		pr_warn("Warning: unknown chip type; assuming OMAP3630ES1.2\n");
 	}
 }
@@ -444,63 +494,11 @@ static void __init omap4_check_revision(void)
 		((omap_rev() >> 12) & 0xf), ((omap_rev() >> 8) & 0xf));
 }

-#define OMAP3_SHOW_FEATURE(feat)		\
-	if (omap3_has_ ##feat())		\
-		printk(#feat" ");
-
-static void __init omap3_cpuinfo(const char *cpu_rev)
-{
-	const char *cpu_name;
-
-	/*
-	 * OMAP3430 and OMAP3530 are assumed to be same.
-	 *
-	 * OMAP3525, OMAP3515 and OMAP3503 can be detected only based
-	 * on available features. Upon detection, update the CPU id
-	 * and CPU class bits.
-	 */
-	if (cpu_is_omap3630()) {
-		cpu_name = "OMAP3630";
-	} else if (cpu_is_omap3517()) {
-		/* AM35xx devices */
-		cpu_name = (omap3_has_sgx()) ? "AM3517" : "AM3505";
-	} else if (cpu_is_ti816x()) {
-		cpu_name = "TI816X";
-	} else if (cpu_is_am335x()) {
-		cpu_name =  "AM335X";
-	} else if (cpu_is_ti814x()) {
-		cpu_name = "TI814X";
-	} else if (omap3_has_iva() && omap3_has_sgx()) {
-		/* OMAP3430, OMAP3525, OMAP3515, OMAP3503 devices */
-		cpu_name = "OMAP3430/3530";
-	} else if (omap3_has_iva()) {
-		cpu_name = "OMAP3525";
-	} else if (omap3_has_sgx()) {
-		cpu_name = "OMAP3515";
-	} else {
-		cpu_name = "OMAP3503";
-	}
-
-	/* Print verbose information */
-	pr_info("%s ES%s (", cpu_name, cpu_rev);
-
-	OMAP3_SHOW_FEATURE(l2cache);
-	OMAP3_SHOW_FEATURE(iva);
-	OMAP3_SHOW_FEATURE(sgx);
-	OMAP3_SHOW_FEATURE(neon);
-	OMAP3_SHOW_FEATURE(isp);
-	OMAP3_SHOW_FEATURE(192mhz_clk);
-
-	printk(")\n");
-}
-
 /*
  * Try to detect the exact revision of the omap we're running on
  */
 void __init omap2_check_revision(void)
 {
-	const char *cpu_rev;
-
 	/*
 	 * At this point we have an idea about the processor revision set
 	 * earlier with omap2_set_globals_tap().
@@ -508,7 +506,7 @@ void __init omap2_check_revision(void)
 	if (cpu_is_omap24xx()) {
 		omap24xx_check_revision();
 	} else if (cpu_is_omap34xx()) {
-		omap3_check_revision(&cpu_rev);
+		omap3_check_revision();

 		/* TI81XX doesn't have feature register */
 		if (!cpu_is_ti81xx())
@@ -516,7 +514,7 @@ void __init omap2_check_revision(void)
 		else
 			ti81xx_check_features();

-		omap3_cpuinfo(cpu_rev);
+		omap3_cpuinfo();
 		return;
 	} else if (cpu_is_omap44xx()) {
 		omap4_check_revision();
--
1.7.0.4

[PATCH-V3 2/2] arm:omap: cleanup & split omap2/3/4_check_revision function

[Vaibhav Hiremath]

This patch doesn't change functionality or behavior of the code
execution; it barely cleans up the code and splits into SoC
specific implementation for Rev ID and feature detection.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
CC: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/id.c              |   46 ++++++--------------------------
 arch/arm/mach-omap2/io.c              |    9 +++++-
 arch/arm/plat-omap/include/plat/cpu.h |    7 ++++-
 3 files changed, 23 insertions(+), 39 deletions(-)

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 752dcae..92e4d55 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -112,7 +112,7 @@ void omap_get_die_id(struct omap_die_id *odi)
 	odi->id_3 = read_tap_reg(OMAP_TAP_DIE_ID_3);
 }

-static void __init omap24xx_check_revision(void)
+void __init omap2xxx_check_revision(void)
 {
 	int i, j;
 	u32 idcode, prod_id;
@@ -222,7 +222,7 @@ static void __init omap3_cpuinfo(void)
 		omap_features |= OMAP3_HAS_ ##feat;			\
 	}

-static void __init omap3_check_features(void)
+void __init omap3xxx_check_features(void)
 {
 	u32 status;

@@ -249,9 +249,11 @@ static void __init omap3_check_features(void)
 	 * TODO: Get additional info (where applicable)
 	 *       e.g. Size of L2 cache.
 	 */
+
+	omap3_cpuinfo();
 }

-static void __init omap4_check_features(void)
+void __init omap4xxx_check_features(void)
 {
 	u32 si_type;

@@ -276,12 +278,13 @@ static void __init omap4_check_features(void)
 	}
 }

-static void __init ti81xx_check_features(void)
+void __init ti81xx_check_features(void)
 {
 	omap_features = OMAP3_HAS_NEON;
+	omap3_cpuinfo();
 }

-static void __init omap3_check_revision(void)
+void __init omap3xxx_check_revision(void)
 {
 	u32 cpuid, idcode;
 	u16 hawkeye;
@@ -421,7 +424,7 @@ static void __init omap3_check_revision(void)
 	}
 }

-static void __init omap4_check_revision(void)
+void __init omap4xxx_check_revision(void)
 {
 	u32 idcode;
 	u16 hawkeye;
@@ -495,37 +498,6 @@ static void __init omap4_check_revision(void)
 }

 /*
- * Try to detect the exact revision of the omap we're running on
- */
-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();
-
-		/* TI81XX doesn't have feature register */
-		if (!cpu_is_ti81xx())
-			omap3_check_features();
-		else
-			ti81xx_check_features();
-
-		omap3_cpuinfo();
-		return;
-	} else if (cpu_is_omap44xx()) {
-		omap4_check_revision();
-		omap4_check_features();
-		return;
-	} else {
-		pr_err("OMAP revision unknown, please fix!\n");
-	}
-}
-
-/*
  * Set up things for map_io and processor detection later on. Gets called
  * pretty much first thing from board init. For multi-omap, this gets
  * cpu_is_omapxxxx() working accurately enough for map_io. Then we'll try to
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 3f174d5..81cf60f 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -351,7 +351,6 @@ static int _set_hwmod_postsetup_state(struct omap_hwmod *oh, void *data)

 static void __init omap_common_init_early(void)
 {
-	omap2_check_revision();
 	omap_init_consistent_dma_size();
 }

@@ -392,6 +391,7 @@ static void __init omap_hwmod_init_postsetup(void)
 void __init omap2420_init_early(void)
 {
 	omap2_set_globals_242x();
+	omap2xxx_check_revision();
 	omap_common_init_early();
 	omap2xxx_voltagedomains_init();
 	omap242x_powerdomains_init();
@@ -404,6 +404,7 @@ void __init omap2420_init_early(void)
 void __init omap2430_init_early(void)
 {
 	omap2_set_globals_243x();
+	omap2xxx_check_revision();
 	omap_common_init_early();
 	omap2xxx_voltagedomains_init();
 	omap243x_powerdomains_init();
@@ -422,6 +423,8 @@ void __init omap2430_init_early(void)
 void __init omap3_init_early(void)
 {
 	omap2_set_globals_3xxx();
+	omap3xxx_check_revision();
+	omap3xxx_check_features();
 	omap_common_init_early();
 	omap3xxx_voltagedomains_init();
 	omap3xxx_powerdomains_init();
@@ -454,6 +457,8 @@ void __init am35xx_init_early(void)
 void __init ti81xx_init_early(void)
 {
 	omap2_set_globals_ti81xx();
+	omap3xxx_check_revision();
+	ti81xx_check_features();
 	omap_common_init_early();
 	omap3xxx_voltagedomains_init();
 	omap3xxx_powerdomains_init();
@@ -468,6 +473,8 @@ void __init ti81xx_init_early(void)
 void __init omap4430_init_early(void)
 {
 	omap2_set_globals_443x();
+	omap4xxx_check_revision();
+	omap4xxx_check_features();
 	omap_common_init_early();
 	omap44xx_voltagedomains_init();
 	omap44xx_powerdomains_init();
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 6b51086..428ccb1 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -451,7 +451,12 @@ IS_OMAP_TYPE(3517, 0x3517)
 #define OMAP447X_CLASS		0x44700044
 #define OMAP4470_REV_ES1_0	(OMAP447X_CLASS | (0x10 << 8))

-void omap2_check_revision(void);
+void omap2xxx_check_revision(void);
+void omap3xxx_check_revision(void);
+void omap4xxx_check_revision(void);
+void omap3xxx_check_features(void);
+void ti81xx_check_features(void);
+void omap4xxx_check_features(void);

 /*
  * Runtime detection of OMAP3 features
--
1.7.0.4

Re: [PATCH-V3 2/2] arm:omap: cleanup & split omap2/3/4_check_revision function

[Tony Lindgren]

* Vaibhav Hiremath <hvaibhav@ti.com> [111219 01:48]:
> This patch doesn't change functionality or behavior of the code
> execution; it barely cleans up the code and splits into SoC
> specific implementation for Rev ID and feature detection.

Thanks, looks good now and allows further clean-up later on.
I'll apply these into soc-part2 branch.

> @@ -495,37 +498,6 @@ static void __init omap4_check_revision(void)
>  }
> 
>  /*
> - * Try to detect the exact revision of the omap we're running on
> - */
> -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();
> -
> -		/* TI81XX doesn't have feature register */
> -		if (!cpu_is_ti81xx())
> -			omap3_check_features();
> -		else
> -			ti81xx_check_features();
> -
> -		omap3_cpuinfo();
> -		return;
> -	} else if (cpu_is_omap44xx()) {
> -		omap4_check_revision();
> -		omap4_check_features();
> -		return;
> -	} else {
> -		pr_err("OMAP revision unknown, please fix!\n");
> -	}
> -}
> -

Nice that we can get rid of this function now :)

Tony

RE: [PATCH-V3 2/2] arm:omap: cleanup & split omap2/3/4_check_revision function

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Tony Lindgren [mailto:tony@atomide.com]
> Sent: Tuesday, December 20, 2011 5:21 AM
> To: Hiremath, Vaibhav
> Cc: linux-omap@vger.kernel.org; Hilman, Kevin
> Subject: Re: [PATCH-V3 2/2] arm:omap: cleanup & split
> omap2/3/4_check_revision function
> 
> * Vaibhav Hiremath <hvaibhav@ti.com> [111219 01:48]:
> > This patch doesn't change functionality or behavior of the code
> > execution; it barely cleans up the code and splits into SoC
> > specific implementation for Rev ID and feature detection.
> 
> Thanks, looks good now and allows further clean-up later on.
> I'll apply these into soc-part2 branch.
> 
Thanks Tony.

Thanks,
Vaibhav


> > @@ -495,37 +498,6 @@ static void __init omap4_check_revision(void)
> >  }
> >
> >  /*
> > - * Try to detect the exact revision of the omap we're running on
> > - */
> > -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();
> > -
> > -		/* TI81XX doesn't have feature register */
> > -		if (!cpu_is_ti81xx())
> > -			omap3_check_features();
> > -		else
> > -			ti81xx_check_features();
> > -
> > -		omap3_cpuinfo();
> > -		return;
> > -	} else if (cpu_is_omap44xx()) {
> > -		omap4_check_revision();
> > -		omap4_check_features();
> > -		return;
> > -	} else {
> > -		pr_err("OMAP revision unknown, please fix!\n");
> > -	}
> > -}
> > -
> 
> Nice that we can get rid of this function now :)
> 
> Tony