[U-Boot] [PATCH] OMAP3: Print correct silicon revision
Premi, Sanjeev
premi at ti.com
Tue Apr 21 20:25:59 CEST 2009
> -----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.
> >
> > 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
> >
> >
More information about the U-Boot
mailing list