[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Sanjeev Premi]

Each call to get_cpu_rev() leads to repetitive
execution of code to detect the cpu revision.

This patchset ensures that mechanism to detect
revision is not executed each time; instead a
stored value is returned.

Since, revision info is needed in s_init(),
the function to identify cpu revision needs
to be called twice. At the moment, it is
necessary/ unavoidable.

Sanjeev Premi (2):
  omap3: Identify the CPU in arch_cpu_init()
  omap3: Identify cpu in s_init()

 cpu/arm_cortexa8/omap3/board.c         |    2 +
 cpu/arm_cortexa8/omap3/sys_info.c      |   73 ++++++++++++++++++++++----------
 include/asm-arm/arch-omap3/sys_proto.h |    3 +-
 include/configs/omap3_beagle.h         |    2 +
 include/configs/omap3_evm.h            |    2 +
 include/configs/omap3_overo.h          |    2 +
 include/configs/omap3_pandora.h        |    2 +
 include/configs/omap3_zoom1.h          |    2 +
 include/configs/omap3_zoom2.h          |    2 +
 9 files changed, 66 insertions(+), 24 deletions(-)

[U-Boot] [PATCH 1/2] omap3: Identify the CPU in arch_cpu_init()

[Sanjeev Premi]

The function get_cpu_rev() is called multiple times
during execution resulting in probe into the IDCODE
register to extract the revision information.

This patch does the following:
- Moves the steps to identify static cpu information
  into arch_cpu_init().
- Updates configs for all omap3 boards to define
  CONFIG_ARCH_CPU_INIT.
- Updates get_cpu_rev() to return value calculated in
  arch_cpu_init().
- Since revision isn't expected to be longer than 8bits,
  get_cpu_rev() return a u8 value instead of u32.

Signed-off-by: Sanjeev Premi <premi@ti.com>
---
 cpu/arm_cortexa8/omap3/sys_info.c      |   60 +++++++++++++++++++------------
 include/asm-arm/arch-omap3/sys_proto.h |    2 +-
 include/configs/omap3_beagle.h         |    2 +
 include/configs/omap3_evm.h            |    2 +
 include/configs/omap3_overo.h          |    2 +
 include/configs/omap3_pandora.h        |    2 +
 include/configs/omap3_zoom1.h          |    2 +
 include/configs/omap3_zoom2.h          |    2 +
 8 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/cpu/arm_cortexa8/omap3/sys_info.c b/cpu/arm_cortexa8/omap3/sys_info.c
index 31b2003..40866ae 100644
--- a/cpu/arm_cortexa8/omap3/sys_info.c
+++ b/cpu/arm_cortexa8/omap3/sys_info.c
@@ -41,6 +41,41 @@ static char *rev_s[CPU_3XX_MAX_REV] = {
 				"3.0",
 				"3.1"};
 
+static u8 cpu_revision;
+
+/**
+ * Perform architecture specific initialization.
+ *
+ * Currently, it identifies the cpu revision.
+ */
+int arch_cpu_init (void)
+{
+	u32 cpuid = 0;
+	struct ctrl_id *id_base;
+
+	/*
+	 * On ES1.0 the IDCODE register is not exposed on L4
+	 * so using CPU ID to differentiate between ES1.0 and > ES1.0.
+	 */
+	__asm__ __volatile__("mrc p15, 0, %0, c0, c0, 0":"=r"(cpuid));
+	if ((cpuid & 0xf) == 0x0) {
+		cpu_revision = CPU_3XX_ES10;
+	} else {
+		/* Decode the IDs on > ES1.0 */
+		id_base = (struct ctrl_id *) OMAP34XX_ID_L4_IO_BASE;
+
+		cpuid = readl(&id_base->idcode);
+
+		cpu_revision = (cpuid >> CPU_3XX_ID_SHIFT) & 0xf;
+
+		/* Some early ES2.0 seem to report rev 0, fix this */
+		if(cpu_revision == 0)
+			cpu_revision = CPU_3XX_ES20;
+	}
+
+	return 0;
+}
+
 /*****************************************************************
  * dieid_num_r(void) - read and set die ID
  *****************************************************************/
@@ -78,30 +113,9 @@ u32 get_cpu_type(void)
 /******************************************
  * get_cpu_rev(void) - extract version info
  ******************************************/
-u32 get_cpu_rev(void)
+u8 get_cpu_rev(void)
 {
-	u32 cpuid = 0;
-	struct ctrl_id *id_base;
-
-	/*
-	 * On ES1.0 the IDCODE register is not exposed on L4
-	 * so using CPU ID to differentiate between ES1.0 and > ES1.0.
-	 */
-	__asm__ __volatile__("mrc p15, 0, %0, c0, c0, 0":"=r"(cpuid));
-	if ((cpuid & 0xf) == 0x0)
-		return CPU_3XX_ES10;
-	else {
-		/* Decode the IDs on > ES1.0 */
-		id_base = (struct ctrl_id *) OMAP34XX_ID_L4_IO_BASE;
-
-		cpuid = (readl(&id_base->idcode) >> CPU_3XX_ID_SHIFT) & 0xf;
-
-		/* Some early ES2.0 seem to report ID 0, fix this */
-		if(cpuid == 0)
-			cpuid = CPU_3XX_ES20;
-
-		return cpuid;
-	}
+	return cpu_revision;
 }
 
 /****************************************************
diff --git a/include/asm-arm/arch-omap3/sys_proto.h b/include/asm-arm/arch-omap3/sys_proto.h
index 34bd515..1c99c45 100644
--- a/include/asm-arm/arch-omap3/sys_proto.h
+++ b/include/asm-arm/arch-omap3/sys_proto.h
@@ -40,7 +40,7 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
 void watchdog_init(void);
 void set_muxconf_regs(void);
 
-u32 get_cpu_rev(void);
+u8 get_cpu_rev(void);
 u32 get_mem_type(void);
 u32 get_sysboot_value(void);
 u32 is_gpmc_muxed(void);
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 19a5ec9..640562c 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -28,6 +28,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_ARCH_CPU_INIT	1
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index a5514ae..0e85393 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -33,6 +33,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_ARCH_CPU_INIT	1
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
index ffb515d..4ff06a3 100644
--- a/include/configs/omap3_overo.h
+++ b/include/configs/omap3_overo.h
@@ -20,6 +20,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_ARCH_CPU_INIT	1
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
index 6f21af3..fee592f 100644
--- a/include/configs/omap3_pandora.h
+++ b/include/configs/omap3_pandora.h
@@ -23,6 +23,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_ARCH_CPU_INIT	1
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/omap3_zoom1.h b/include/configs/omap3_zoom1.h
index da4b677..8866cda 100644
--- a/include/configs/omap3_zoom1.h
+++ b/include/configs/omap3_zoom1.h
@@ -29,6 +29,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_ARCH_CPU_INIT	1
+
 /*
  * High Level Configuration Options
  */
diff --git a/include/configs/omap3_zoom2.h b/include/configs/omap3_zoom2.h
index 32cd6fd..5381aa9 100644
--- a/include/configs/omap3_zoom2.h
+++ b/include/configs/omap3_zoom2.h
@@ -30,6 +30,8 @@
 #ifndef __CONFIG_H
 #define __CONFIG_H
 
+#define CONFIG_ARCH_CPU_INIT	1
+
 /*
  * High Level Configuration Options
  */
-- 
1.6.2.2

[U-Boot] [PATCH 2/2] omap3: Identify cpu in s_init()

[Sanjeev Premi]

When s_init() is called, the silicon version hasn't yet
been identified. This would lead to incorrect index in
the DPLL table.

This patch ensures that silicon is identified as first
step in s_init().

When called from s_init(), the globals updated in the
function identify_cpu() lie in 'relocated' address
space.

So, identify_cpu() is called again in arch_cpu_init().

Signed-off-by: Sanjeev Premi <premi@ti.com>
---
 cpu/arm_cortexa8/omap3/board.c         |    2 ++
 cpu/arm_cortexa8/omap3/sys_info.c      |   17 +++++++++++++++--
 include/asm-arm/arch-omap3/sys_proto.h |    1 +
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/cpu/arm_cortexa8/omap3/board.c b/cpu/arm_cortexa8/omap3/board.c
index 2aa69b3..2f27cb9 100644
--- a/cpu/arm_cortexa8/omap3/board.c
+++ b/cpu/arm_cortexa8/omap3/board.c
@@ -193,6 +193,8 @@ void s_init(void)
 {
 	int in_sdram = is_running_in_sdram();
 
+	identify_cpu();
+
 	watchdog_init();
 
 	try_unlock_memory();
diff --git a/cpu/arm_cortexa8/omap3/sys_info.c b/cpu/arm_cortexa8/omap3/sys_info.c
index 40866ae..449262a 100644
--- a/cpu/arm_cortexa8/omap3/sys_info.c
+++ b/cpu/arm_cortexa8/omap3/sys_info.c
@@ -44,11 +44,11 @@ static char *rev_s[CPU_3XX_MAX_REV] = {
 static u8 cpu_revision;
 
 /**
- * Perform architecture specific initialization.
+ * Identify the silicon
  *
  * Currently, it identifies the cpu revision.
  */
-int arch_cpu_init (void)
+void identify_cpu (void)
 {
 	u32 cpuid = 0;
 	struct ctrl_id *id_base;
@@ -72,6 +72,19 @@ int arch_cpu_init (void)
 		if(cpu_revision == 0)
 			cpu_revision = CPU_3XX_ES20;
 	}
+}
+
+/**
+ * Perform architecture specific initialization
+ */
+int arch_cpu_init (void)
+{
+	/*
+	 * The identification done in s_init is 'lost' due to
+	 * relocation. The variable contents are not same.
+	 * Function identify_cpu() is called again.
+	 */
+	identify_cpu();
 
 	return 0;
 }
diff --git a/include/asm-arm/arch-omap3/sys_proto.h b/include/asm-arm/arch-omap3/sys_proto.h
index 1c99c45..9ddd272 100644
--- a/include/asm-arm/arch-omap3/sys_proto.h
+++ b/include/asm-arm/arch-omap3/sys_proto.h
@@ -40,6 +40,7 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
 void watchdog_init(void);
 void set_muxconf_regs(void);
 
+void identify_cpu(void);
 u8 get_cpu_rev(void);
 u32 get_mem_type(void);
 u32 get_sysboot_value(void);
-- 
1.6.2.2

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Tom]

Sanjeev Premi wrote:
> Each call to get_cpu_rev() leads to repetitive
> execution of code to detect the cpu revision.
> 
> This patchset ensures that mechanism to detect
> revision is not executed each time; instead a
> stored value is returned.
> 
> Since, revision info is needed in s_init(),
> the function to identify cpu revision needs
> to be called twice. At the moment, it is
> necessary/ unavoidable.
> 

Is there some other reason to read this register only once?
This function is not used frequently and i do not
think the complexity of optimizing is necessary.

Tom

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Tom [mailto:Tom.Rix at windriver.com] 
> Sent: Tuesday, December 15, 2009 10:44 PM
> To: Premi, Sanjeev
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection 
> of cpu revision
> 
> Sanjeev Premi wrote:
> > Each call to get_cpu_rev() leads to repetitive
> > execution of code to detect the cpu revision.
> > 
> > This patchset ensures that mechanism to detect
> > revision is not executed each time; instead a
> > stored value is returned.
> > 
> > Since, revision info is needed in s_init(),
> > the function to identify cpu revision needs
> > to be called twice. At the moment, it is
> > necessary/ unavoidable.
> > 
> 
> Is there some other reason to read this register only once?
> This function is not used frequently and i do not
> think the complexity of optimizing is necessary.

As more processors and revision specific code gets added,
there will be more occurrences for this check.

I have just posted patches for basic support for the
AM35x processors. Now, the cpurev for this silicon is
ES1.0; but have otherwise similar features that the
OMAP35x ES3.1 si (I am not accounting other IP differences
between the processors here).

Also, I don't believe there is any complexity added as
the contents of register are being read and saved in a
global variable for use later.

Best regards,
Sanjeev
> 
> Tom
> 

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Wolfgang Denk]

Dear "Premi, Sanjeev",

In message <B85A65D85D7EB246BE421B3FB0FBB59301E157AAE8@dbde02.ent.ti.com> you wrote:
> 
> Also, I don't believe there is any complexity added as
> the contents of register are being read and saved in a
> global variable for use later.

Global variables are a bad thing if there is not really a good reason
to hav ethem. Here it makes no sense to me. Execution time seems
uncritical, and there is no kind of hardware wear involved with
readin the registers, so like Tom I don't see a reason for this
"optimization".

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How does a project get to be a year late?      ... One day at a time.

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Premi, Sanjeev 
> Sent: Tuesday, December 15, 2009 6:48 PM
> To: u-boot at lists.denx.de
> Cc: Premi, Sanjeev
> Subject: [PATCH 0/2] omap3: Optimize detection of cpu revision
> 
> Each call to get_cpu_rev() leads to repetitive
> execution of code to detect the cpu revision.
> 
> This patchset ensures that mechanism to detect
> revision is not executed each time; instead a
> stored value is returned.
> 
> Since, revision info is needed in s_init(),
> the function to identify cpu revision needs
> to be called twice. At the moment, it is
> necessary/ unavoidable.
> 
> Sanjeev Premi (2):
>   omap3: Identify the CPU in arch_cpu_init()
>   omap3: Identify cpu in s_init()
> 
>  cpu/arm_cortexa8/omap3/board.c         |    2 +
>  cpu/arm_cortexa8/omap3/sys_info.c      |   73 
> ++++++++++++++++++++++----------
>  include/asm-arm/arch-omap3/sys_proto.h |    3 +-
>  include/configs/omap3_beagle.h         |    2 +
>  include/configs/omap3_evm.h            |    2 +
>  include/configs/omap3_overo.h          |    2 +
>  include/configs/omap3_pandora.h        |    2 +
>  include/configs/omap3_zoom1.h          |    2 +
>  include/configs/omap3_zoom2.h          |    2 +
>  9 files changed, 66 insertions(+), 24 deletions(-)
> 
> 

Sandeep, Tom,

Any comments on this series on your queue..

Best regards,
Sanjeev

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Paulraj, Sandeep]

> 
> > -----Original Message-----
> > From: Premi, Sanjeev
> > Sent: Tuesday, December 15, 2009 6:48 PM
> > To: u-boot at lists.denx.de
> > Cc: Premi, Sanjeev
> > Subject: [PATCH 0/2] omap3: Optimize detection of cpu revision
> >
> > Each call to get_cpu_rev() leads to repetitive
> > execution of code to detect the cpu revision.
> >
> > This patchset ensures that mechanism to detect
> > revision is not executed each time; instead a
> > stored value is returned.
> >
> > Since, revision info is needed in s_init(),
> > the function to identify cpu revision needs
> > to be called twice. At the moment, it is
> > necessary/ unavoidable.
> >
> > Sanjeev Premi (2):
> >   omap3: Identify the CPU in arch_cpu_init()
> >   omap3: Identify cpu in s_init()
> >
> >  cpu/arm_cortexa8/omap3/board.c         |    2 +
> >  cpu/arm_cortexa8/omap3/sys_info.c      |   73
> > ++++++++++++++++++++++----------
> >  include/asm-arm/arch-omap3/sys_proto.h |    3 +-
> >  include/configs/omap3_beagle.h         |    2 +
> >  include/configs/omap3_evm.h            |    2 +
> >  include/configs/omap3_overo.h          |    2 +
> >  include/configs/omap3_pandora.h        |    2 +
> >  include/configs/omap3_zoom1.h          |    2 +
> >  include/configs/omap3_zoom2.h          |    2 +
> >  9 files changed, 66 insertions(+), 24 deletions(-)
> >
> >
> 
> Sandeep, Tom,
> 
> Any comments on this series on your queue..

Sanjeev,

Wolfgang had some comments on this.

http://www.mail-archive.com/u-boot at lists.denx.de/msg26568.html

> 
> Best regards,
> Sanjeev
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Paulraj, Sandeep 
> Sent: Thursday, January 07, 2010 9:02 PM
> To: Premi, Sanjeev; u-boot at lists.denx.de
> Subject: RE: [PATCH 0/2] omap3: Optimize detection of cpu revision
> 
> 
> > 
> > > -----Original Message-----
> > > From: Premi, Sanjeev
> > > Sent: Tuesday, December 15, 2009 6:48 PM
> > > To: u-boot at lists.denx.de
> > > Cc: Premi, Sanjeev
> > > Subject: [PATCH 0/2] omap3: Optimize detection of cpu revision
> > >
> > > Each call to get_cpu_rev() leads to repetitive
> > > execution of code to detect the cpu revision.
> > >
> > > This patchset ensures that mechanism to detect
> > > revision is not executed each time; instead a
> > > stored value is returned.
> > >
> > > Since, revision info is needed in s_init(),
> > > the function to identify cpu revision needs
> > > to be called twice. At the moment, it is
> > > necessary/ unavoidable.
> > >
> > > Sanjeev Premi (2):
> > >   omap3: Identify the CPU in arch_cpu_init()
> > >   omap3: Identify cpu in s_init()
> > >
> > >  cpu/arm_cortexa8/omap3/board.c         |    2 +
> > >  cpu/arm_cortexa8/omap3/sys_info.c      |   73
> > > ++++++++++++++++++++++----------
> > >  include/asm-arm/arch-omap3/sys_proto.h |    3 +-
> > >  include/configs/omap3_beagle.h         |    2 +
> > >  include/configs/omap3_evm.h            |    2 +
> > >  include/configs/omap3_overo.h          |    2 +
> > >  include/configs/omap3_pandora.h        |    2 +
> > >  include/configs/omap3_zoom1.h          |    2 +
> > >  include/configs/omap3_zoom2.h          |    2 +
> > >  9 files changed, 66 insertions(+), 24 deletions(-)
> > >
> > >
> > 
> > Sandeep, Tom,
> > 
> > Any comments on this series on your queue..
> 
> Sanjeev,
> 
> Wolfgang had some comments on this.
> 
> http://www.mail-archive.com/u-boot at lists.denx.de/msg26568.html

Sorry missed that one.
Will respond.

~sanjeev

> 
> > 
> > Best regards,
> > Sanjeev
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Paulraj, Sandeep 
> Sent: Thursday, January 07, 2010 9:02 PM
> To: Premi, Sanjeev; u-boot at lists.denx.de
> Subject: RE: [PATCH 0/2] omap3: Optimize detection of cpu revision
> > 
> > > -----Original Message-----
> > > From: Premi, Sanjeev
> > > Sent: Tuesday, December 15, 2009 6:48 PM
> > > To: u-boot at lists.denx.de
> > > Cc: Premi, Sanjeev
> > > Subject: [PATCH 0/2] omap3: Optimize detection of cpu revision
> > >
> > > Each call to get_cpu_rev() leads to repetitive
> > > execution of code to detect the cpu revision.
> > >
> > > This patchset ensures that mechanism to detect
> > > revision is not executed each time; instead a
> > > stored value is returned.
> > >
> > > Since, revision info is needed in s_init(),
> > > the function to identify cpu revision needs
> > > to be called twice. At the moment, it is
> > > necessary/ unavoidable.
> > >
> > > Sanjeev Premi (2):
> > >   omap3: Identify the CPU in arch_cpu_init()
> > >   omap3: Identify cpu in s_init()
> > >
> > >  cpu/arm_cortexa8/omap3/board.c         |    2 +
> > >  cpu/arm_cortexa8/omap3/sys_info.c      |   73
> > > ++++++++++++++++++++++----------
> > >  include/asm-arm/arch-omap3/sys_proto.h |    3 +-
> > >  include/configs/omap3_beagle.h         |    2 +
> > >  include/configs/omap3_evm.h            |    2 +
> > >  include/configs/omap3_overo.h          |    2 +
> > >  include/configs/omap3_pandora.h        |    2 +
> > >  include/configs/omap3_zoom1.h          |    2 +
> > >  include/configs/omap3_zoom2.h          |    2 +
> > >  9 files changed, 66 insertions(+), 24 deletions(-)
> > >
> > >
> > 
> > Sandeep, Tom,
> > 
> > Any comments on this series on your queue..
> 
> Sanjeev,
> 
> Wolfgang had some comments on this.
> 
> http://www.mail-archive.com/u-boot at lists.denx.de/msg26568.html
> 

Did not find this mail in my inbox (may be reason to miss it earlier).
Anyway, pasting it below to maintain context:

> Dear "Premi, Sanjeev",
> 
> In message <b85a65d85d7eb246be421b3fb0fbb59301e157a...@dbde02.ent.ti.com>
> you wrote:
> > 
> > Also, I don't believe there is any complexity added as
> > the contents of register are being read and saved in a
> > global variable for use later.
> 
> Global variables are a bad thing if there is not really a good reason
> to hav ethem. Here it makes no sense to me. Execution time seems
> uncritical, and there is no kind of hardware wear involved with
> readin the registers, so like Tom I don't see a reason for this
> "optimization".

Tom, Denx,

As this patch stands, there isn't much code to optimize; but the
change was meant as enabler for the next set of processors. The
register and mechanism is same ...just interpretation will differ.

There is already a patchset for AM35x devices and there will new
patches for OMAP36x. 

Also, I believe faster execution time is always better; not just
in critical sections of code. I possibly used "global" quite loosely;
while responding earlier. The variable cpu_revision (being discussed)
here is actually a 'static'. (See patch 1/2).

But, if we feel otherwise, I can revert to executing detection
mechanism each time in the function.

However, are their any comments on remainder of the patch e.g.
moving the cpu identification eary in the u-boot exectuion. The
DPLL settings etc will depend upon the si identification.

Best regards,
Sanjeev

> 
> Best regards,
> Wolfgang Denk

Best regards,
Sanjeev

> > 
> > Best regards,
> > Sanjeev
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Tom]

Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Paulraj, Sandeep 
>> Sent: Thursday, January 07, 2010 9:02 PM
>> To: Premi, Sanjeev; u-boot at lists.denx.de
>> Subject: RE: [PATCH 0/2] omap3: Optimize detection of cpu revision
>>>> -----Original Message-----
>>>> From: Premi, Sanjeev
>>>> Sent: Tuesday, December 15, 2009 6:48 PM
>>>> To: u-boot at lists.denx.de
>>>> Cc: Premi, Sanjeev
>>>> Subject: [PATCH 0/2] omap3: Optimize detection of cpu revision
>>>>
>>>> Each call to get_cpu_rev() leads to repetitive
>>>> execution of code to detect the cpu revision.
>>>>
>>>> This patchset ensures that mechanism to detect
>>>> revision is not executed each time; instead a
>>>> stored value is returned.
>>>>
>>>> Since, revision info is needed in s_init(),
>>>> the function to identify cpu revision needs
>>>> to be called twice. At the moment, it is
>>>> necessary/ unavoidable.
>>>>
>>>> Sanjeev Premi (2):
>>>>   omap3: Identify the CPU in arch_cpu_init()
>>>>   omap3: Identify cpu in s_init()
>>>>
>>>>  cpu/arm_cortexa8/omap3/board.c         |    2 +
>>>>  cpu/arm_cortexa8/omap3/sys_info.c      |   73
>>>> ++++++++++++++++++++++----------
>>>>  include/asm-arm/arch-omap3/sys_proto.h |    3 +-
>>>>  include/configs/omap3_beagle.h         |    2 +
>>>>  include/configs/omap3_evm.h            |    2 +
>>>>  include/configs/omap3_overo.h          |    2 +
>>>>  include/configs/omap3_pandora.h        |    2 +
>>>>  include/configs/omap3_zoom1.h          |    2 +
>>>>  include/configs/omap3_zoom2.h          |    2 +
>>>>  9 files changed, 66 insertions(+), 24 deletions(-)
>>>>
>>>>
>>> Sandeep, Tom,
>>>
>>> Any comments on this series on your queue..
>> Sanjeev,
>>
>> Wolfgang had some comments on this.
>>
>> http://www.mail-archive.com/u-boot at lists.denx.de/msg26568.html
>>
> 
> Did not find this mail in my inbox (may be reason to miss it earlier).
> Anyway, pasting it below to maintain context:
> 
>> Dear "Premi, Sanjeev",
>>
>> In message <b85a65d85d7eb246be421b3fb0fbb59301e157a...@dbde02.ent.ti.com>
>> you wrote:
>>> Also, I don't believe there is any complexity added as
>>> the contents of register are being read and saved in a
>>> global variable for use later.
>> Global variables are a bad thing if there is not really a good reason
>> to hav ethem. Here it makes no sense to me. Execution time seems
>> uncritical, and there is no kind of hardware wear involved with
>> readin the registers, so like Tom I don't see a reason for this
>> "optimization".
> 
> Tom, Denx,
> 
> As this patch stands, there isn't much code to optimize; but the
> change was meant as enabler for the next set of processors. The
> register and mechanism is same ...just interpretation will differ.
> 
> There is already a patchset for AM35x devices and there will new
> patches for OMAP36x. 
> 
> Also, I believe faster execution time is always better; not just
> in critical sections of code. I possibly used "global" quite loosely;
> while responding earlier. The variable cpu_revision (being discussed)
> here is actually a 'static'. (See patch 1/2).
> 
> But, if we feel otherwise, I can revert to executing detection
> mechanism each time in the function.
> 
> However, are their any comments on remainder of the patch e.g.
> moving the cpu identification eary in the u-boot exectuion. The
> DPLL settings etc will depend upon the si identification.
> 

I am not in favor of the patch.
Please remove it and rework your patchset.
Tom

> Best regards,
> Sanjeev
> 
>> Best regards,
>> Wolfgang Denk
> 
> Best regards,
> Sanjeev
> 
>>> Best regards,
>>> Sanjeev
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Wolfgang Denk]

Dear Sanjeev,

in message <B85A65D85D7EB246BE421B3FB0FBB59301E1EB4A5D@dbde02.ent.ti.com> you wrote:
>
> Also, I believe faster execution time is always better; not just
> in critical sections of code. I possibly used "global" quite loosely;
> while responding earlier. The variable cpu_revision (being discussed)
> here is actually a 'static'. (See patch 1/2).

So how much faster is reading this variable compared to reading from
the register?

Can you really measure _any_ difference?

> But, if we feel otherwise, I can revert to executing detection
> mechanism each time in the function.

Please undo this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ernest asks Frank how long he has been working for the company.
        "Ever since they threatened to fire me."