Premi, Sanjeev wrote: >> -----Original Message----- >> From: Dirk Behme [mailto:dirk.be...@googlemail.com] >> Sent: Tuesday, April 21, 2009 10:26 PM >> To: Premi, Sanjeev >> Cc: u-boot@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 <pr...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot