Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
Dear Sanjeev, in message b85a65d85d7eb246be421b3fb0fbb59301e1eb4...@dbde02.ent.ti.com you wrote: 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). So how much faster is reading this variable compared to reading from the register? Can you really measure _any_ difference? But, if we feel otherwise, I can revert to executing detection mechanism each time in the function. Please undo this. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Ernest asks Frank how long he has been working for the company. Ever since they threatened to fire me. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
Premi, Sanjeev wrote: -Original Message- From: Paulraj, Sandeep Sent: Thursday, January 07, 2010 9:02 PM To: Premi, Sanjeev; u-boot@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@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...@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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
-Original Message- From: Paulraj, Sandeep Sent: Thursday, January 07, 2010 9:02 PM To: Premi, Sanjeev; u-boot@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@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...@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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
-Original Message- From: Paulraj, Sandeep Sent: Thursday, January 07, 2010 9:02 PM To: Premi, Sanjeev; u-boot@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@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 Sorry missed that one. Will respond. ~sanjeev Best regards, Sanjeev ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [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@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.. Best regards, Sanjeev ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [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@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 Best regards, Sanjeev ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
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. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de How does a project get to be a year late? ... One day at a time. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [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(-) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
Sanjeev Premi wrote: 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. Is there some other reason to read this register only once? This function is not used frequently and i do not think the complexity of optimizing is necessary. Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision
-Original Message- From: Tom [mailto:tom@windriver.com] Sent: Tuesday, December 15, 2009 10:44 PM To: Premi, Sanjeev Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH 0/2] omap3: Optimize detection of cpu revision Sanjeev Premi wrote: 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. Is there some other reason to read this register only once? This function is not used frequently and i do not think the complexity of optimizing is necessary. As more processors and revision specific code gets added, there will be more occurrences for this check. I have just posted patches for basic support for the AM35x processors. Now, the cpurev for this silicon is ES1.0; but have otherwise similar features that the OMAP35x ES3.1 si (I am not accounting other IP differences between the processors here). 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. Best regards, Sanjeev Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
