> -----Original Message----- > From: Premi, Sanjeev > Sent: Tuesday, April 21, 2009 11:37 PM > To: 'Dirk Behme' > Cc: u-boot@lists.denx.de > Subject: RE: [U-Boot] [PATCH] OMAP3: Print correct silicon revision > > > > -----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; > } >
...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. > > > > 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 <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? 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 > > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot