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

[Dirk Behme <dirk.behme@googlemail.com>]

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