From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Tue, 21 Apr 2009 21:34:23 +0200 Subject: [U-Boot] [PATCH] OMAP3: Print correct silicon revision In-Reply-To: References: <1240331018-12710-1-git-send-email-premi@ti.com> <49EDFA9B.6010702@googlemail.com> Message-ID: <49EE1FBF.1070608@googlemail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>> --- >>>> 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