> -----Original Message-----
> From: Dirk Behme [mailto:dirk.be...@googlemail.com]
> Sent: Wednesday, April 22, 2009 1:04 AM
> To: Premi, Sanjeev
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] OMAP3: Print correct silicon revision
>
> Dear Premi,
>
> Premi, Sanjeev wrote:
> >> -----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.
>
> What's about something like in the attachment? Compile tested
> only. Do
> you like to test it?
Sure. Will do it in the morning...
I did make few changes since your last mail as well.
Will post the updates later in the day.
~sanjeev
>
> 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 <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
