[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Sanjeev Premi]

The function display_board_info() displays the silicon
revision as 2 - based on the return value from get_cpu_rev().

This is incorrect as the current Si version is 3.1

This patch displays the correct version; but does not
change get_cpu_rev() to minimize the code impact.

Signed-off-by: Sanjeev Premi <premi@ti.com>
---
 cpu/arm_cortexa8/omap3/sys_info.c |   37 +++++++++++++++++++++++++++++++++++--
 1 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/cpu/arm_cortexa8/omap3/sys_info.c b/cpu/arm_cortexa8/omap3/sys_info.c
index b385b91..8c6a4d6 100644
--- a/cpu/arm_cortexa8/omap3/sys_info.c
+++ b/cpu/arm_cortexa8/omap3/sys_info.c
@@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
 static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
 static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
 
+static char omap_revision[8] = "";
+
 /*****************************************************************
  * dieid_num_r(void) - read and set die ID
  *****************************************************************/
@@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
 
 }
 
+/**
+ * Converts cpu revision into a string
+ */
+void set_omap_revision(void)
+{
+	u32 idcode;
+	ctrl_id_t *id_base;
+	char *str_rev = &omap_revision[0];
+
+	if (get_cpu_rev() == CPU_3430_ES1) {
+		strcat (str_rev, "ES1.0");
+	}
+	else {
+		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
+
+		idcode = readl(&id_base->idcode);
+
+		if (idcode == 0x1B7AE02F)
+			strcat (str_rev, "ES2.0");
+		else if (idcode == 0x2B7AE02F)
+			strcat (str_rev, "ES2.1");
+		else if (idcode == 0x3B7AE02F)
+			strcat (str_rev, "ES3.0");
+		else if (idcode == 0x4B7AE02F)
+			strcat (str_rev, "ES3.1");
+		else
+			strcat (str_rev, "ES??");
+	}
+}
+
 /****************************************************
  * is_mem_sdr() - return 1 if mem type in use is SDR
  ****************************************************/
@@ -232,9 +264,10 @@ void display_board_info(u32 btype)
 		sec_s = "?";
 	}
 
+	set_omap_revision();
 
-	printf("OMAP%s-%s rev %d, CPU-OPP2 L3-165MHz\n", cpu_s,
-	       sec_s, get_cpu_rev());
+	printf("OMAP%s-%s (%s), CPU-OPP2 L3-165MHz\n", cpu_s,
+	       sec_s, omap_revision);
 	printf("%s + %s/%s\n", sysinfo.board_string,
 	       mem_s, sysinfo.nand_string);
 
-- 
1.6.2.1

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Dirk Behme]

Dear Premi,

Sanjeev Premi wrote:
> The function display_board_info() displays the silicon
> revision as 2 - based on the return value from get_cpu_rev().
> 
> This is incorrect as the current Si version is 3.1

Thanks for the patch and fixing this!

> This patch displays the correct version; but does not
> change get_cpu_rev() to minimize the code impact.

I wonder if it wouldn't be better (and cleaner) to fix get_cpu_rev()?

A quick grep resulted in 5 (?) locations which might be affected:

./cpu/arm_cortexa8/cpu.c:104:   if (get_cpu_rev() == CPU_3430_ES2) { 

./cpu/arm_cortexa8/cpu.c:134:   if (get_cpu_rev() == CPU_3430_ES2) { 

./cpu/arm_cortexa8/omap3/clock.c:173:   sil_index = get_cpu_rev() - 1; 

./cpu/arm_cortexa8/omap3/sys_info.c:144:        if (get_cpu_rev() == 
CPU_3430_ES2)
./cpu/arm_cortexa8/omap3/sys_info.c:237:               sec_s, 
get_cpu_rev());

If we extend the existing macros

#define CPU_3430_ES1		1
#define CPU_3430_ES2		2

to e.g.

#define CPU_3430_ES10		1
#define CPU_3430_ES20		2
#define CPU_3430_ES21		3
#define CPU_3430_ES30		4
#define CPU_3430_ES31		5

then the three

== CPU_3430_ES2

will simply become

 >= CPU_3430_ES20

The sil_index = get_cpu_rev() - 1;  needs a deeper look, though.

Regarding the ASCII strings: With the numbers get_cpu_rev()  returns 
we then could index a const struct with the ASCII strings for the 
revision print. E.g.

printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);

What do you think?

> Signed-off-by: Sanjeev Premi <premi@ti.com>
> ---
>  cpu/arm_cortexa8/omap3/sys_info.c |   37 +++++++++++++++++++++++++++++++++++--
>  1 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c b/cpu/arm_cortexa8/omap3/sys_info.c
> index b385b91..8c6a4d6 100644
> --- a/cpu/arm_cortexa8/omap3/sys_info.c
> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>  
> +static char omap_revision[8] = "";
> +
>  /*****************************************************************
>   * dieid_num_r(void) - read and set die ID
>   *****************************************************************/
> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>  
>  }
>  
> +/**
> + * Converts cpu revision into a string
> + */
> +void set_omap_revision(void)
> +{
> +	u32 idcode;
> +	ctrl_id_t *id_base;
> +	char *str_rev = &omap_revision[0];
> +
> +	if (get_cpu_rev() == CPU_3430_ES1) {
> +		strcat (str_rev, "ES1.0");
> +	}
> +	else {
> +		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> +
> +		idcode = readl(&id_base->idcode);
> +
> +		if (idcode == 0x1B7AE02F)
> +			strcat (str_rev, "ES2.0");
> +		else if (idcode == 0x2B7AE02F)
> +			strcat (str_rev, "ES2.1");
> +		else if (idcode == 0x3B7AE02F)
> +			strcat (str_rev, "ES3.0");
> +		else if (idcode == 0x4B7AE02F)

It looks to me that only the highest nibble of idcode changes here? 
Maybe we could better mask & shift it a little and create a nice macro 
for it?

Best regards

Dirk

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
> Sent: Tuesday, April 21, 2009 10:26 PM
> To: Premi, Sanjeev
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> 
> Dear Premi,
> 
> Sanjeev Premi wrote:
> > The function display_board_info() displays the silicon
> > revision as 2 - based on the return value from get_cpu_rev().
> > 
> > This is incorrect as the current Si version is 3.1
> 
> Thanks for the patch and fixing this!
> 
> > This patch displays the correct version; but does not
> > change get_cpu_rev() to minimize the code impact.
> 
> I wonder if it wouldn't be better (and cleaner) to fix get_cpu_rev()?

Yes. This is what I started with; but then this is where I felt that
fix may run 'deeper"

u32 get_board_type(void)
{
	if (get_cpu_rev() == CPU_3430_ES2)
		return sysinfo.board_type_v2;
	else
		return sysinfo.board_type_v1;
}

I couldn't figure out how this impacts boards other than the EVM.

> 
> A quick grep resulted in 5 (?) locations which might be affected:
> 
> ./cpu/arm_cortexa8/cpu.c:104:   if (get_cpu_rev() == CPU_3430_ES2) { 
> 
> ./cpu/arm_cortexa8/cpu.c:134:   if (get_cpu_rev() == CPU_3430_ES2) { 
> 
> ./cpu/arm_cortexa8/omap3/clock.c:173:   sil_index = 
> get_cpu_rev() - 1; 
> 
> ./cpu/arm_cortexa8/omap3/sys_info.c:144:        if (get_cpu_rev() == 
> CPU_3430_ES2)
> ./cpu/arm_cortexa8/omap3/sys_info.c:237:               sec_s, 
> get_cpu_rev());
> 
> If we extend the existing macros
> 
> #define CPU_3430_ES1		1
> #define CPU_3430_ES2		2
> 
> to e.g.
> 
> #define CPU_3430_ES10		1
> #define CPU_3430_ES20		2
> #define CPU_3430_ES21		3
> #define CPU_3430_ES30		4
> #define CPU_3430_ES31		5
> 
> then the three
> 
> == CPU_3430_ES2
> 
> will simply become
> 
>  >= CPU_3430_ES20
> 
> The sil_index = get_cpu_rev() - 1;  needs a deeper look, though.
> 
> Regarding the ASCII strings: With the numbers get_cpu_rev()  returns 
> we then could index a const struct with the ASCII strings for the 
> revision print. E.g.
> 
> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
> 
> What do you think?
> 
> > Signed-off-by: Sanjeev Premi <premi@ti.com>
> > ---
> >  cpu/arm_cortexa8/omap3/sys_info.c |   37 
> +++++++++++++++++++++++++++++++++++--
> >  1 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/cpu/arm_cortexa8/omap3/sys_info.c 
> b/cpu/arm_cortexa8/omap3/sys_info.c
> > index b385b91..8c6a4d6 100644
> > --- a/cpu/arm_cortexa8/omap3/sys_info.c
> > +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> > @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = 
> (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
> >  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> >  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
> >  
> > +static char omap_revision[8] = "";
> > +
> >  /*****************************************************************
> >   * dieid_num_r(void) - read and set die ID
> >   *****************************************************************/
> > @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
> >  
> >  }
> >  
> > +/**
> > + * Converts cpu revision into a string
> > + */
> > +void set_omap_revision(void)
> > +{
> > +	u32 idcode;
> > +	ctrl_id_t *id_base;
> > +	char *str_rev = &omap_revision[0];
> > +
> > +	if (get_cpu_rev() == CPU_3430_ES1) {
> > +		strcat (str_rev, "ES1.0");
> > +	}
> > +	else {
> > +		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> > +
> > +		idcode = readl(&id_base->idcode);
> > +
> > +		if (idcode == 0x1B7AE02F)
> > +			strcat (str_rev, "ES2.0");
> > +		else if (idcode == 0x2B7AE02F)
> > +			strcat (str_rev, "ES2.1");
> > +		else if (idcode == 0x3B7AE02F)
> > +			strcat (str_rev, "ES3.0");
> > +		else if (idcode == 0x4B7AE02F)
> 
> It looks to me that only the highest nibble of idcode changes here? 
> Maybe we could better mask & shift it a little and create a 
> nice macro 
> for it?
> 
> Best regards
> 
> Dirk
> 
> 

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Premi, Sanjeev 
> Sent: Tuesday, April 21, 2009 11:37 PM
> To: 'Dirk Behme'
> Cc: u-boot at lists.denx.de
> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> 
> 
> > -----Original Message-----
> > From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
> > Sent: Tuesday, April 21, 2009 10:26 PM
> > To: Premi, Sanjeev
> > Cc: u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> > 
> > Dear Premi,
> > 
> > Sanjeev Premi wrote:
> > > The function display_board_info() displays the silicon
> > > revision as 2 - based on the return value from get_cpu_rev().
> > > 
> > > This is incorrect as the current Si version is 3.1
> > 
> > Thanks for the patch and fixing this!
> > 
> > > This patch displays the correct version; but does not
> > > change get_cpu_rev() to minimize the code impact.
> > 
> > I wonder if it wouldn't be better (and cleaner) to fix 
> get_cpu_rev()?
> 
> Yes. This is what I started with; but then this is where I felt that
> fix may run 'deeper"
> 
> u32 get_board_type(void)
> {
> 	if (get_cpu_rev() == CPU_3430_ES2)
> 		return sysinfo.board_type_v2;
> 	else
> 		return sysinfo.board_type_v1;
> }
> 

...sorry, mail 'went' before I wanted to!

> I couldn't figure out how this impacts boards other than the EVM.

Though I admit not having much time looking for the impact. Beyond
this, I believe the fix could be straight forward.

> > 
> > A quick grep resulted in 5 (?) locations which might be affected:
> > 
> > ./cpu/arm_cortexa8/cpu.c:104:   if (get_cpu_rev() == 
> CPU_3430_ES2) { 
> > 
> > ./cpu/arm_cortexa8/cpu.c:134:   if (get_cpu_rev() == 
> CPU_3430_ES2) { 
> > 
> > ./cpu/arm_cortexa8/omap3/clock.c:173:   sil_index = 
> > get_cpu_rev() - 1; 
> > 
> > ./cpu/arm_cortexa8/omap3/sys_info.c:144:        if 
> (get_cpu_rev() == 
> > CPU_3430_ES2)
> > ./cpu/arm_cortexa8/omap3/sys_info.c:237:               sec_s, 
> > get_cpu_rev());
> > 
> > If we extend the existing macros
> > 
> > #define CPU_3430_ES1		1
> > #define CPU_3430_ES2		2
> > 
> > to e.g.
> > 
> > #define CPU_3430_ES10		1
> > #define CPU_3430_ES20		2
> > #define CPU_3430_ES21		3
> > #define CPU_3430_ES30		4
> > #define CPU_3430_ES31		5
> > 
> > then the three
> > 
> > == CPU_3430_ES2
> > 
> > will simply become
> > 
> >  >= CPU_3430_ES20

There seems to be a slight differene between the silicon
revision between 34x and 35x for the highest nibble value
for early si revs - ES 1.0 and ES2.0.

> > 
> > The sil_index = get_cpu_rev() - 1;  needs a deeper look, though.
> > 
> > Regarding the ASCII strings: With the numbers get_cpu_rev() 
>  returns 
> > we then could index a const struct with the ASCII strings for the 
> > revision print. E.g.
> > 
> > printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
> > 
> > What do you think?
> > 
> > > Signed-off-by: Sanjeev Premi <premi@ti.com>
> > > ---
> > >  cpu/arm_cortexa8/omap3/sys_info.c |   37 
> > +++++++++++++++++++++++++++++++++++--
> > >  1 files changed, 35 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/cpu/arm_cortexa8/omap3/sys_info.c 
> > b/cpu/arm_cortexa8/omap3/sys_info.c
> > > index b385b91..8c6a4d6 100644
> > > --- a/cpu/arm_cortexa8/omap3/sys_info.c
> > > +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> > > @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = 
> > (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
> > >  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> > >  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
> > >  
> > > +static char omap_revision[8] = "";
> > > +
> > >  
> /*****************************************************************
> > >   * dieid_num_r(void) - read and set die ID
> > >   
> *****************************************************************/
> > > @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
> > >  
> > >  }
> > >  
> > > +/**
> > > + * Converts cpu revision into a string
> > > + */
> > > +void set_omap_revision(void)
> > > +{
> > > +	u32 idcode;
> > > +	ctrl_id_t *id_base;
> > > +	char *str_rev = &omap_revision[0];
> > > +
> > > +	if (get_cpu_rev() == CPU_3430_ES1) {
> > > +		strcat (str_rev, "ES1.0");
> > > +	}
> > > +	else {
> > > +		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> > > +
> > > +		idcode = readl(&id_base->idcode);
> > > +
> > > +		if (idcode == 0x1B7AE02F)
> > > +			strcat (str_rev, "ES2.0");
> > > +		else if (idcode == 0x2B7AE02F)
> > > +			strcat (str_rev, "ES2.1");
> > > +		else if (idcode == 0x3B7AE02F)
> > > +			strcat (str_rev, "ES3.0");
> > > +		else if (idcode == 0x4B7AE02F)
> > 
> > It looks to me that only the highest nibble of idcode changes here? 
> > Maybe we could better mask & shift it a little and create a 
> > nice macro 
> > for it?

It is already done in the kernel; but I am not sure if we could save
much - unless we use the index as you suggest above.

> > 
> > Best regards
> > 
> > Dirk
> > 
> > 

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Dirk Behme]

Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
>> Sent: Tuesday, April 21, 2009 10:26 PM
>> To: Premi, Sanjeev
>> Cc: u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>>
>> Dear Premi,
>>
>> Sanjeev Premi wrote:
>>> The function display_board_info() displays the silicon
>>> revision as 2 - based on the return value from get_cpu_rev().
>>>
>>> This is incorrect as the current Si version is 3.1
>> Thanks for the patch and fixing this!
>>
>>> This patch displays the correct version; but does not
>>> change get_cpu_rev() to minimize the code impact.
>> I wonder if it wouldn't be better (and cleaner) to fix get_cpu_rev()?
> 
> Yes. This is what I started with; but then this is where I felt that
> fix may run 'deeper"
> 
> u32 get_board_type(void)
> {
> 	if (get_cpu_rev() == CPU_3430_ES2)
> 		return sysinfo.board_type_v2;
> 	else
> 		return sysinfo.board_type_v1;
> }
> 
> I couldn't figure out how this impacts boards other than the EVM.

Maybe I missed something, but independent of what this function does, 
if we replace

if (get_cpu_rev() == CPU_3430_ES2)

with

if (get_cpu_rev() >= CPU_3430_ES20)

the functionality of this function (i.e. the value returned) wouldn't 
change compared to what it actually returns?

Best regards

Dirk

>> A quick grep resulted in 5 (?) locations which might be affected:
>>
>> ./cpu/arm_cortexa8/cpu.c:104:   if (get_cpu_rev() == CPU_3430_ES2) { 
>>
>> ./cpu/arm_cortexa8/cpu.c:134:   if (get_cpu_rev() == CPU_3430_ES2) { 
>>
>> ./cpu/arm_cortexa8/omap3/clock.c:173:   sil_index = 
>> get_cpu_rev() - 1; 
>>
>> ./cpu/arm_cortexa8/omap3/sys_info.c:144:        if (get_cpu_rev() == 
>> CPU_3430_ES2)
>> ./cpu/arm_cortexa8/omap3/sys_info.c:237:               sec_s, 
>> get_cpu_rev());
>>
>> If we extend the existing macros
>>
>> #define CPU_3430_ES1		1
>> #define CPU_3430_ES2		2
>>
>> to e.g.
>>
>> #define CPU_3430_ES10		1
>> #define CPU_3430_ES20		2
>> #define CPU_3430_ES21		3
>> #define CPU_3430_ES30		4
>> #define CPU_3430_ES31		5
>>
>> then the three
>>
>> == CPU_3430_ES2
>>
>> will simply become
>>
>>  >= CPU_3430_ES20
>>
>> The sil_index = get_cpu_rev() - 1;  needs a deeper look, though.
>>
>> Regarding the ASCII strings: With the numbers get_cpu_rev()  returns 
>> we then could index a const struct with the ASCII strings for the 
>> revision print. E.g.
>>
>> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
>>
>> What do you think?
>>
>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>> ---
>>>  cpu/arm_cortexa8/omap3/sys_info.c |   37 
>> +++++++++++++++++++++++++++++++++++--
>>>  1 files changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c 
>> b/cpu/arm_cortexa8/omap3/sys_info.c
>>> index b385b91..8c6a4d6 100644
>>> --- a/cpu/arm_cortexa8/omap3/sys_info.c
>>> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
>>> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = 
>> (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
>>>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>>>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>>>  
>>> +static char omap_revision[8] = "";
>>> +
>>>  /*****************************************************************
>>>   * dieid_num_r(void) - read and set die ID
>>>   *****************************************************************/
>>> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>>>  
>>>  }
>>>  
>>> +/**
>>> + * Converts cpu revision into a string
>>> + */
>>> +void set_omap_revision(void)
>>> +{
>>> +	u32 idcode;
>>> +	ctrl_id_t *id_base;
>>> +	char *str_rev = &omap_revision[0];
>>> +
>>> +	if (get_cpu_rev() == CPU_3430_ES1) {
>>> +		strcat (str_rev, "ES1.0");
>>> +	}
>>> +	else {
>>> +		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
>>> +
>>> +		idcode = readl(&id_base->idcode);
>>> +
>>> +		if (idcode == 0x1B7AE02F)
>>> +			strcat (str_rev, "ES2.0");
>>> +		else if (idcode == 0x2B7AE02F)
>>> +			strcat (str_rev, "ES2.1");
>>> +		else if (idcode == 0x3B7AE02F)
>>> +			strcat (str_rev, "ES3.0");
>>> +		else if (idcode == 0x4B7AE02F)
>> It looks to me that only the highest nibble of idcode changes here? 
>> Maybe we could better mask & shift it a little and create a 
>> nice macro 
>> for it?
>>
>> Best regards
>>
>> Dirk
>>
>>

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Dirk Behme]

Dear Premi,

Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Premi, Sanjeev 
>> Sent: Tuesday, April 21, 2009 11:37 PM
>> To: 'Dirk Behme'
>> Cc: u-boot at lists.denx.de
>> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>>
>>
>>> -----Original Message-----
>>> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
>>> Sent: Tuesday, April 21, 2009 10:26 PM
>>> To: Premi, Sanjeev
>>> Cc: u-boot at lists.denx.de
>>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>>>
>>> Dear Premi,
>>>
>>> Sanjeev Premi wrote:
>>>> The function display_board_info() displays the silicon
>>>> revision as 2 - based on the return value from get_cpu_rev().
>>>>
>>>> This is incorrect as the current Si version is 3.1
>>> Thanks for the patch and fixing this!
>>>
>>>> This patch displays the correct version; but does not
>>>> change get_cpu_rev() to minimize the code impact.
>>> I wonder if it wouldn't be better (and cleaner) to fix 
>> get_cpu_rev()?
>>
>> Yes. This is what I started with; but then this is where I felt that
>> fix may run 'deeper"
>>
>> u32 get_board_type(void)
>> {
>> 	if (get_cpu_rev() == CPU_3430_ES2)
>> 		return sysinfo.board_type_v2;
>> 	else
>> 		return sysinfo.board_type_v1;
>> }
>>
> 
> ...sorry, mail 'went' before I wanted to!
> 
>> I couldn't figure out how this impacts boards other than the EVM.
> 
> Though I admit not having much time looking for the impact. Beyond
> this, I believe the fix could be straight forward.

What's about something like in the attachment? Compile tested only. Do 
you like to test it?

Best regards

Dirk

>>> A quick grep resulted in 5 (?) locations which might be affected:
>>>
>>> ./cpu/arm_cortexa8/cpu.c:104:   if (get_cpu_rev() == 
>> CPU_3430_ES2) { 
>>> ./cpu/arm_cortexa8/cpu.c:134:   if (get_cpu_rev() == 
>> CPU_3430_ES2) { 
>>> ./cpu/arm_cortexa8/omap3/clock.c:173:   sil_index = 
>>> get_cpu_rev() - 1; 
>>>
>>> ./cpu/arm_cortexa8/omap3/sys_info.c:144:        if 
>> (get_cpu_rev() == 
>>> CPU_3430_ES2)
>>> ./cpu/arm_cortexa8/omap3/sys_info.c:237:               sec_s, 
>>> get_cpu_rev());
>>>
>>> If we extend the existing macros
>>>
>>> #define CPU_3430_ES1		1
>>> #define CPU_3430_ES2		2
>>>
>>> to e.g.
>>>
>>> #define CPU_3430_ES10		1
>>> #define CPU_3430_ES20		2
>>> #define CPU_3430_ES21		3
>>> #define CPU_3430_ES30		4
>>> #define CPU_3430_ES31		5
>>>
>>> then the three
>>>
>>> == CPU_3430_ES2
>>>
>>> will simply become
>>>
>>>  >= CPU_3430_ES20
> 
> There seems to be a slight differene between the silicon
> revision between 34x and 35x for the highest nibble value
> for early si revs - ES 1.0 and ES2.0.
> 
>>> The sil_index = get_cpu_rev() - 1;  needs a deeper look, though.
>>>
>>> Regarding the ASCII strings: With the numbers get_cpu_rev() 
>>  returns 
>>> we then could index a const struct with the ASCII strings for the 
>>> revision print. E.g.
>>>
>>> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
>>>
>>> What do you think?
>>>
>>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>>>> ---
>>>>  cpu/arm_cortexa8/omap3/sys_info.c |   37 
>>> +++++++++++++++++++++++++++++++++++--
>>>>  1 files changed, 35 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c 
>>> b/cpu/arm_cortexa8/omap3/sys_info.c
>>>> index b385b91..8c6a4d6 100644
>>>> --- a/cpu/arm_cortexa8/omap3/sys_info.c
>>>> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
>>>> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = 
>>> (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
>>>>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>>>>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>>>>  
>>>> +static char omap_revision[8] = "";
>>>> +
>>>>  
>> /*****************************************************************
>>>>   * dieid_num_r(void) - read and set die ID
>>>>   
>> *****************************************************************/
>>>> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>>>>  
>>>>  }
>>>>  
>>>> +/**
>>>> + * Converts cpu revision into a string
>>>> + */
>>>> +void set_omap_revision(void)
>>>> +{
>>>> +	u32 idcode;
>>>> +	ctrl_id_t *id_base;
>>>> +	char *str_rev = &omap_revision[0];
>>>> +
>>>> +	if (get_cpu_rev() == CPU_3430_ES1) {
>>>> +		strcat (str_rev, "ES1.0");
>>>> +	}
>>>> +	else {
>>>> +		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
>>>> +
>>>> +		idcode = readl(&id_base->idcode);
>>>> +
>>>> +		if (idcode == 0x1B7AE02F)
>>>> +			strcat (str_rev, "ES2.0");
>>>> +		else if (idcode == 0x2B7AE02F)
>>>> +			strcat (str_rev, "ES2.1");
>>>> +		else if (idcode == 0x3B7AE02F)
>>>> +			strcat (str_rev, "ES3.0");
>>>> +		else if (idcode == 0x4B7AE02F)
>>> It looks to me that only the highest nibble of idcode changes here? 
>>> Maybe we could better mask & shift it a little and create a 
>>> nice macro 
>>> for it?
> 
> It is already done in the kernel; but I am not sure if we could save
> much - unless we use the index as you suggest above.
> 
>>> Best regards
>>>
>>> Dirk
>>>
>>>

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: omap3_revision.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20090421/bdcb6b06/attachment-0001.txt 

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
> Sent: Wednesday, April 22, 2009 1:04 AM
> To: Premi, Sanjeev
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> 
> Dear Premi,
> 
> Premi, Sanjeev wrote:
> >> -----Original Message-----
> >> From: Premi, Sanjeev 
> >> Sent: Tuesday, April 21, 2009 11:37 PM
> >> To: 'Dirk Behme'
> >> Cc: u-boot at lists.denx.de
> >> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> >>
> >>
> >>> -----Original Message-----
> >>> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
> >>> Sent: Tuesday, April 21, 2009 10:26 PM
> >>> To: Premi, Sanjeev
> >>> Cc: u-boot at lists.denx.de
> >>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct 
> silicon revision
> >>>
> >>> Dear Premi,
> >>>
> >>> Sanjeev Premi wrote:
> >>>> The function display_board_info() displays the silicon
> >>>> revision as 2 - based on the return value from get_cpu_rev().
> >>>>
> >>>> This is incorrect as the current Si version is 3.1
> >>> Thanks for the patch and fixing this!
> >>>
> >>>> This patch displays the correct version; but does not
> >>>> change get_cpu_rev() to minimize the code impact.
> >>> I wonder if it wouldn't be better (and cleaner) to fix 
> >> get_cpu_rev()?
> >>
> >> Yes. This is what I started with; but then this is where I 
> felt that
> >> fix may run 'deeper"
> >>
> >> u32 get_board_type(void)
> >> {
> >> 	if (get_cpu_rev() == CPU_3430_ES2)
> >> 		return sysinfo.board_type_v2;
> >> 	else
> >> 		return sysinfo.board_type_v1;
> >> }
> >>
> > 
> > ...sorry, mail 'went' before I wanted to!
> > 
> >> I couldn't figure out how this impacts boards other than the EVM.
> > 
> > Though I admit not having much time looking for the impact. Beyond
> > this, I believe the fix could be straight forward.
> 
> What's about something like in the attachment? Compile tested 
> only. Do 
> you like to test it?

Sure. Will do it in the morning...

I did make few changes since your last mail as well.
Will post the updates later in the day.

~sanjeev

> 
> Best regards
> 
> Dirk

> 
> >>> A quick grep resulted in 5 (?) locations which might be affected:
> >>>
> >>> ./cpu/arm_cortexa8/cpu.c:104:   if (get_cpu_rev() == 
> >> CPU_3430_ES2) { 
> >>> ./cpu/arm_cortexa8/cpu.c:134:   if (get_cpu_rev() == 
> >> CPU_3430_ES2) { 
> >>> ./cpu/arm_cortexa8/omap3/clock.c:173:   sil_index = 
> >>> get_cpu_rev() - 1; 
> >>>
> >>> ./cpu/arm_cortexa8/omap3/sys_info.c:144:        if 
> >> (get_cpu_rev() == 
> >>> CPU_3430_ES2)
> >>> ./cpu/arm_cortexa8/omap3/sys_info.c:237:               sec_s, 
> >>> get_cpu_rev());
> >>>
> >>> If we extend the existing macros
> >>>
> >>> #define CPU_3430_ES1		1
> >>> #define CPU_3430_ES2		2
> >>>
> >>> to e.g.
> >>>
> >>> #define CPU_3430_ES10		1
> >>> #define CPU_3430_ES20		2
> >>> #define CPU_3430_ES21		3
> >>> #define CPU_3430_ES30		4
> >>> #define CPU_3430_ES31		5
> >>>
> >>> then the three
> >>>
> >>> == CPU_3430_ES2
> >>>
> >>> will simply become
> >>>
> >>>  >= CPU_3430_ES20
> > 
> > There seems to be a slight differene between the silicon
> > revision between 34x and 35x for the highest nibble value
> > for early si revs - ES 1.0 and ES2.0.
> > 
> >>> The sil_index = get_cpu_rev() - 1;  needs a deeper look, though.
> >>>
> >>> Regarding the ASCII strings: With the numbers get_cpu_rev() 
> >>  returns 
> >>> we then could index a const struct with the ASCII strings for the 
> >>> revision print. E.g.
> >>>
> >>> printf(" ... %s ...", ... omap_revision[get_cpu_rev()] ...);
> >>>
> >>> What do you think?
> >>>
> >>>> Signed-off-by: Sanjeev Premi <premi@ti.com>
> >>>> ---
> >>>>  cpu/arm_cortexa8/omap3/sys_info.c |   37 
> >>> +++++++++++++++++++++++++++++++++++--
> >>>>  1 files changed, 35 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c 
> >>> b/cpu/arm_cortexa8/omap3/sys_info.c
> >>>> index b385b91..8c6a4d6 100644
> >>>> --- a/cpu/arm_cortexa8/omap3/sys_info.c
> >>>> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> >>>> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = 
> >>> (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
> >>>>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
> >>>>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
> >>>>  
> >>>> +static char omap_revision[8] = "";
> >>>> +
> >>>>  
> >> /*****************************************************************
> >>>>   * dieid_num_r(void) - read and set die ID
> >>>>   
> >> *****************************************************************/
> >>>> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
> >>>>  
> >>>>  }
> >>>>  
> >>>> +/**
> >>>> + * Converts cpu revision into a string
> >>>> + */
> >>>> +void set_omap_revision(void)
> >>>> +{
> >>>> +	u32 idcode;
> >>>> +	ctrl_id_t *id_base;
> >>>> +	char *str_rev = &omap_revision[0];
> >>>> +
> >>>> +	if (get_cpu_rev() == CPU_3430_ES1) {
> >>>> +		strcat (str_rev, "ES1.0");
> >>>> +	}
> >>>> +	else {
> >>>> +		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> >>>> +
> >>>> +		idcode = readl(&id_base->idcode);
> >>>> +
> >>>> +		if (idcode == 0x1B7AE02F)
> >>>> +			strcat (str_rev, "ES2.0");
> >>>> +		else if (idcode == 0x2B7AE02F)
> >>>> +			strcat (str_rev, "ES2.1");
> >>>> +		else if (idcode == 0x3B7AE02F)
> >>>> +			strcat (str_rev, "ES3.0");
> >>>> +		else if (idcode == 0x4B7AE02F)
> >>> It looks to me that only the highest nibble of idcode 
> changes here? 
> >>> Maybe we could better mask & shift it a little and create a 
> >>> nice macro 
> >>> for it?
> > 
> > It is already done in the kernel; but I am not sure if we could save
> > much - unless we use the index as you suggest above.
> > 
> >>> Best regards
> >>>
> >>> Dirk
> >>>
> >>>
> 
> 

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
> Sent: Wednesday, April 22, 2009 1:04 AM
> To: Premi, Sanjeev
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> 
> Dear Premi,
> 
> Premi, Sanjeev wrote:
> >> -----Original Message-----
> >> From: Premi, Sanjeev 
> >> Sent: Tuesday, April 21, 2009 11:37 PM
> >> To: 'Dirk Behme'
> >> Cc: u-boot at lists.denx.de
> >> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
> >>
> >>
> >>> -----Original Message-----
> >>> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
> >>> Sent: Tuesday, April 21, 2009 10:26 PM
> >>> To: Premi, Sanjeev
> >>> Cc: u-boot at lists.denx.de
> >>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct 
> silicon revision
> >>>
> >>> Dear Premi,
> >>>
> >>> Sanjeev Premi wrote:
> >>>> The function display_board_info() displays the silicon
> >>>> revision as 2 - based on the return value from get_cpu_rev().
> >>>>
> >>>> This is incorrect as the current Si version is 3.1
> >>> Thanks for the patch and fixing this!
> >>>
> >>>> This patch displays the correct version; but does not
> >>>> change get_cpu_rev() to minimize the code impact.
> >>> I wonder if it wouldn't be better (and cleaner) to fix 
> >> get_cpu_rev()?
> >>
> >> Yes. This is what I started with; but then this is where I 
> felt that
> >> fix may run 'deeper"
> >>
> >> u32 get_board_type(void)
> >> {
> >> 	if (get_cpu_rev() == CPU_3430_ES2)
> >> 		return sysinfo.board_type_v2;
> >> 	else
> >> 		return sysinfo.board_type_v1;
> >> }
> >>
> > 
> > ...sorry, mail 'went' before I wanted to!
> > 
> >> I couldn't figure out how this impacts boards other than the EVM.
> > 
> > Though I admit not having much time looking for the impact. Beyond
> > this, I believe the fix could be straight forward.
> 
> What's about something like in the attachment? Compile tested 
> only. Do 
> you like to test it?

Yes, this works on the EVM.

I did spend some more time & fouund that value from get_board_type is
ignored in the display_board_info().

I will submit a patch to remove this function if it is really not needed.

Best regards,
Sanjeev

> 
> Best regards
> 
> Dirk
> 
[...snip...]

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Dirk Behme]

Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
>> Sent: Wednesday, April 22, 2009 1:04 AM
>> To: Premi, Sanjeev
>> Cc: u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>>
>> Dear Premi,
>>
>> Premi, Sanjeev wrote:
>>>> -----Original Message-----
>>>> From: Premi, Sanjeev 
>>>> Sent: Tuesday, April 21, 2009 11:37 PM
>>>> To: 'Dirk Behme'
>>>> Cc: u-boot at lists.denx.de
>>>> Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Dirk Behme [mailto:dirk.behme at googlemail.com] 
>>>>> Sent: Tuesday, April 21, 2009 10:26 PM
>>>>> To: Premi, Sanjeev
>>>>> Cc: u-boot at lists.denx.de
>>>>> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct 
>> silicon revision
>>>>> Dear Premi,
>>>>>
>>>>> Sanjeev Premi wrote:
>>>>>> The function display_board_info() displays the silicon
>>>>>> revision as 2 - based on the return value from get_cpu_rev().
>>>>>>
>>>>>> This is incorrect as the current Si version is 3.1
>>>>> Thanks for the patch and fixing this!
>>>>>
>>>>>> This patch displays the correct version; but does not
>>>>>> change get_cpu_rev() to minimize the code impact.
>>>>> I wonder if it wouldn't be better (and cleaner) to fix 
>>>> get_cpu_rev()?
>>>>
>>>> Yes. This is what I started with; but then this is where I 
>> felt that
>>>> fix may run 'deeper"
>>>>
>>>> u32 get_board_type(void)
>>>> {
>>>> 	if (get_cpu_rev() == CPU_3430_ES2)
>>>> 		return sysinfo.board_type_v2;
>>>> 	else
>>>> 		return sysinfo.board_type_v1;
>>>> }
>>>>
>>> ...sorry, mail 'went' before I wanted to!
>>>
>>>> I couldn't figure out how this impacts boards other than the EVM.
>>> Though I admit not having much time looking for the impact. Beyond
>>> this, I believe the fix could be straight forward.
>> What's about something like in the attachment? Compile tested 
>> only. Do 
>> you like to test it?
> 
> Yes, this works on the EVM.

Great, thanks for testing!

> I did spend some more time & fouund that value from get_board_type is
> ignored in the display_board_info().

Hmm, yes, good catch.

> I will submit a patch to remove this function if it is really not needed.

Ok.

Best regards

Dirk

Btw.: Updated patch in attachment, maybe it helps you. Tested on Beagle.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: omap3_revision.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20090422/34f85878/attachment-0001.txt 

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Jean-Christophe PLAGNIOL-VILLARD]

On 21:53 Tue 21 Apr     , Sanjeev Premi wrote:
> The function display_board_info() displays the silicon
> revision as 2 - based on the return value from get_cpu_rev().
> 
> This is incorrect as the current Si version is 3.1
> 
> This patch displays the correct version; but does not
> change get_cpu_rev() to minimize the code impact.
> 
> Signed-off-by: Sanjeev Premi <premi@ti.com>
> ---
>  cpu/arm_cortexa8/omap3/sys_info.c |   37 +++++++++++++++++++++++++++++++++++--
>  1 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c b/cpu/arm_cortexa8/omap3/sys_info.c
> index b385b91..8c6a4d6 100644
> --- a/cpu/arm_cortexa8/omap3/sys_info.c
> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>  
> +static char omap_revision[8] = "";
why 8?
you only have 5 chars
so
static char omap_revision[6] = "";
will be enough
> +
>  /*****************************************************************
>   * dieid_num_r(void) - read and set die ID
>   *****************************************************************/
> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>  
>  }
>  
why not this
in the cpu header

#define ES20	1
#define ES21	2
#define ES30	3
#define ES31	4
> +/**
> + * Converts cpu revision into a string
> + */
> +void set_omap_revision(void)
char* get_str_omap_revision(void)
{
	u32 idcode;
	ctrl_id_t *id_base;

	if (omap_revision[0] != '\0')
		return omap_revision;

	if (get_cpu_rev() == CPU_3430_ES1) {
		strcat (omap_revision, "ES1.0");
	}
	else {
		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;

		idcode = readl(&id_base->idcode) >> 28;

		switch(idcode) {
		case ES20:
			strcat (omap_revision, "ES2.0");
			break;
		case ES21:
			strcat (omap_revision, "ES2.1");
			break;
		case ES30:
			strcat (omap_revision, "ES3.0");
			break;
		case ES31:
			strcat (omap_revision, "ES3.1");
			break;
		default:
			strcat (str_rev, "ES??");
	}
	return omap_revision;
}

Best Regards,
J.

[U-Boot] [PATCH] OMAP3: Print correct silicon revision

[Dirk Behme]

Dear Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:53 Tue 21 Apr     , Sanjeev Premi wrote:
>> The function display_board_info() displays the silicon
>> revision as 2 - based on the return value from get_cpu_rev().
>>
>> This is incorrect as the current Si version is 3.1
>>
>> This patch displays the correct version; but does not
>> change get_cpu_rev() to minimize the code impact.
>>
>> Signed-off-by: Sanjeev Premi <premi@ti.com>
>> ---
>>  cpu/arm_cortexa8/omap3/sys_info.c |   37 +++++++++++++++++++++++++++++++++++--
>>  1 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpu/arm_cortexa8/omap3/sys_info.c b/cpu/arm_cortexa8/omap3/sys_info.c
>> index b385b91..8c6a4d6 100644
>> --- a/cpu/arm_cortexa8/omap3/sys_info.c
>> +++ b/cpu/arm_cortexa8/omap3/sys_info.c
>> @@ -36,6 +36,8 @@ static gpmc_csx_t *gpmc_cs_base = (gpmc_csx_t *)GPMC_CONFIG_CS0_BASE;
>>  static sdrc_t *sdrc_base = (sdrc_t *)OMAP34XX_SDRC_BASE;
>>  static ctrl_t *ctrl_base = (ctrl_t *)OMAP34XX_CTRL_BASE;
>>  
>> +static char omap_revision[8] = "";
> why 8?
> you only have 5 chars
> so
> static char omap_revision[6] = "";
> will be enough
>> +
>>  /*****************************************************************
>>   * dieid_num_r(void) - read and set die ID
>>   *****************************************************************/
>> @@ -90,6 +92,36 @@ u32 get_cpu_rev(void)
>>  
>>  }
>>  
> why not this
> in the cpu header
> 
> #define ES20	1
> #define ES21	2
> #define ES30	3
> #define ES31	4
>> +/**
>> + * Converts cpu revision into a string
>> + */
>> +void set_omap_revision(void)
> char* get_str_omap_revision(void)
> {
> 	u32 idcode;
> 	ctrl_id_t *id_base;
> 
> 	if (omap_revision[0] != '\0')
> 		return omap_revision;
> 
> 	if (get_cpu_rev() == CPU_3430_ES1) {
> 		strcat (omap_revision, "ES1.0");
> 	}
> 	else {
> 		id_base = (ctrl_id_t *)OMAP34XX_ID_L4_IO_BASE;
> 
> 		idcode = readl(&id_base->idcode) >> 28;
> 
> 		switch(idcode) {
> 		case ES20:
> 			strcat (omap_revision, "ES2.0");
> 			break;
> 		case ES21:
> 			strcat (omap_revision, "ES2.1");
> 			break;
> 		case ES30:
> 			strcat (omap_revision, "ES3.0");
> 			break;
> 		case ES31:
> 			strcat (omap_revision, "ES3.1");
> 			break;
> 		default:
> 			strcat (str_rev, "ES??");
> 	}
> 	return omap_revision;
> }

Did you notice that we had some discussion about this and that Premi 
will send an completely redone patch, soon?

http://lists.denx.de/pipermail/u-boot/2009-April/051186.html
http://lists.denx.de/pipermail/u-boot/2009-April/051187.html
http://lists.denx.de/pipermail/u-boot/2009-April/051191.html
http://lists.denx.de/pipermail/u-boot/2009-April/051189.html
http://lists.denx.de/pipermail/u-boot/2009-April/051193.html
http://lists.denx.de/pipermail/u-boot/2009-April/051194.html
http://lists.denx.de/pipermail/u-boot/2009-April/051228.html
http://lists.denx.de/pipermail/u-boot/2009-April/051241.html

Dirk