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