[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 at 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