[U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision

[Premi, Sanjeev]

> -----Original Message-----
> From: Paulraj, Sandeep 
> Sent: Thursday, January 07, 2010 9:02 PM
> To: Premi, Sanjeev; u-boot at lists.denx.de
> Subject: RE: [PATCH 0/2] omap3: Optimize detection of cpu revision
> > 
> > > -----Original Message-----
> > > From: Premi, Sanjeev
> > > Sent: Tuesday, December 15, 2009 6:48 PM
> > > To: u-boot at lists.denx.de
> > > Cc: Premi, Sanjeev
> > > Subject: [PATCH 0/2] omap3: Optimize detection of cpu revision
> > >
> > > Each call to get_cpu_rev() leads to repetitive
> > > execution of code to detect the cpu revision.
> > >
> > > This patchset ensures that mechanism to detect
> > > revision is not executed each time; instead a
> > > stored value is returned.
> > >
> > > Since, revision info is needed in s_init(),
> > > the function to identify cpu revision needs
> > > to be called twice. At the moment, it is
> > > necessary/ unavoidable.
> > >
> > > Sanjeev Premi (2):
> > >   omap3: Identify the CPU in arch_cpu_init()
> > >   omap3: Identify cpu in s_init()
> > >
> > >  cpu/arm_cortexa8/omap3/board.c         |    2 +
> > >  cpu/arm_cortexa8/omap3/sys_info.c      |   73
> > > ++++++++++++++++++++++----------
> > >  include/asm-arm/arch-omap3/sys_proto.h |    3 +-
> > >  include/configs/omap3_beagle.h         |    2 +
> > >  include/configs/omap3_evm.h            |    2 +
> > >  include/configs/omap3_overo.h          |    2 +
> > >  include/configs/omap3_pandora.h        |    2 +
> > >  include/configs/omap3_zoom1.h          |    2 +
> > >  include/configs/omap3_zoom2.h          |    2 +
> > >  9 files changed, 66 insertions(+), 24 deletions(-)
> > >
> > >
> > 
> > Sandeep, Tom,
> > 
> > Any comments on this series on your queue..
> 
> Sanjeev,
> 
> Wolfgang had some comments on this.
> 
> http://www.mail-archive.com/u-boot@lists.denx.de/msg26568.html
> 

Did not find this mail in my inbox (may be reason to miss it earlier).
Anyway, pasting it below to maintain context:

> Dear "Premi, Sanjeev",
> 
> In message <b85a65d85d7eb246be421b3fb0fbb59301e157a... at dbde02.ent.ti.com>
> you wrote:
> > 
> > Also, I don't believe there is any complexity added as
> > the contents of register are being read and saved in a
> > global variable for use later.
> 
> Global variables are a bad thing if there is not really a good reason
> to hav ethem. Here it makes no sense to me. Execution time seems
> uncritical, and there is no kind of hardware wear involved with
> readin the registers, so like Tom I don't see a reason for this
> "optimization".

Tom, Denx,

As this patch stands, there isn't much code to optimize; but the
change was meant as enabler for the next set of processors. The
register and mechanism is same ...just interpretation will differ.

There is already a patchset for AM35x devices and there will new
patches for OMAP36x. 

Also, I believe faster execution time is always better; not just
in critical sections of code. I possibly used "global" quite loosely;
while responding earlier. The variable cpu_revision (being discussed)
here is actually a 'static'. (See patch 1/2).

But, if we feel otherwise, I can revert to executing detection
mechanism each time in the function.

However, are their any comments on remainder of the patch e.g.
moving the cpu identification eary in the u-boot exectuion. The
DPLL settings etc will depend upon the si identification.

Best regards,
Sanjeev

> 
> Best regards,
> Wolfgang Denk

Best regards,
Sanjeev

> > 
> > Best regards,
> > Sanjeev
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
>