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

[Tom <Tom.Rix@windriver.com>]

Premi, Sanjeev wrote:
>> -----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 at 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...@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.
> 

I am not in favor of the patch.
Please remove it and rework your patchset.
Tom

> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot