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

[Premi, Sanjeev]

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

> 
> 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 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?
> 
> Best regards
> 
> Dirk
> 
>