Patchwork [U-Boot] omap4: board: change global data pointer to file scope

login
register
mail settings
Submitter Nishanth Menon
Date Oct. 25, 2010, 8:13 p.m.
Message ID <1288037639-20704-1-git-send-email-nm@ti.com>
Download mbox | patch
Permalink /patch/71927/
State Awaiting Upstream
Delegated to: Sandeep Paulraj
Headers show

Comments

Nishanth Menon - Oct. 25, 2010, 8:13 p.m.
DECLARE_GLOBAL_DATA_PTR is currently defined within the scope
of function while it is a global pointer. Change the scope of
definition to replicate it's global scope. This seems to help
gcc 4.5 optimizations as well.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/cpu/armv7/omap4/board.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Nishanth Menon - Nov. 4, 2010, 3:58 p.m.
On Mon, Oct 25, 2010 at 16:13, Menon, Nishanth <nm@ti.com> wrote:
> DECLARE_GLOBAL_DATA_PTR is currently defined within the scope
> of function while it is a global pointer. Change the scope of
> definition to replicate it's global scope. This seems to help
> gcc 4.5 optimizations as well.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

gentle ping - I dont see comments on this, is it ok to pull this in?

> ---
>  arch/arm/cpu/armv7/omap4/board.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/omap4/board.c
> b/arch/arm/cpu/armv7/omap4/board.c
> index 24a66f5..f967d09 100644
> --- a/arch/arm/cpu/armv7/omap4/board.c
> +++ b/arch/arm/cpu/armv7/omap4/board.c
> @@ -32,6 +32,8 @@
>  #include <asm/arch/sys_proto.h>
>  #include <asm/sizes.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  /*
>  * Routine: s_init
>  * Description: Does early system init of muxing and clocks.
> @@ -100,7 +102,6 @@ u32 sdram_size(void)
>  */
>  int dram_init(void)
>  {
> -       DECLARE_GLOBAL_DATA_PTR;
>
>  #if defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
>        gd->bd->bi_dram[0].start = 0x80000000;
> --
> 1.6.3.3
>
>
Wolfgang Denk - Nov. 4, 2010, 9:26 p.m.
Dear "Menon, Nishanth",

In message <AANLkTi=HWBUi5=zcfumMA-eAY7AAqw10g73w_cEx8sM5@mail.gmail.com> you wrote:
> On Mon, Oct 25, 2010 at 16:13, Menon, Nishanth <nm@ti.com> wrote:
> > DECLARE_GLOBAL_DATA_PTR is currently defined within the scope
> > of function while it is a global pointer. Change the scope of
> > definition to replicate it's global scope. This seems to help
> > gcc 4.5 optimizations as well.
> >
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> 
> gentle ping - I dont see comments on this, is it ok to pull this in?

This is OK.

The use of DECLARE_GLOBAL_DATA_PTR at function scope (instead file
scope) is known to cause incorrect code in some compiler /
architecture versions. We consider it a bug.


Best regards,

Wolfgang Denk
Steve Sakoman - Nov. 4, 2010, 9:29 p.m.
On Thu, Nov 4, 2010 at 8:58 AM, Menon, Nishanth <nm@ti.com> wrote:
> On Mon, Oct 25, 2010 at 16:13, Menon, Nishanth <nm@ti.com> wrote:
>> DECLARE_GLOBAL_DATA_PTR is currently defined within the scope
>> of function while it is a global pointer. Change the scope of
>> definition to replicate it's global scope. This seems to help
>> gcc 4.5 optimizations as well.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>
> gentle ping - I dont see comments on this, is it ok to pull this in?

Tested on Panda, no issues found.

Tested-by: Steve Sakoman <steve.sakoman@linaro.org>

Steve
Sandeep Paulraj - Nov. 19, 2010, 4:23 p.m.
> 
> DECLARE_GLOBAL_DATA_PTR is currently defined within the scope
> of function while it is a global pointer. Change the scope of
> definition to replicate it's global scope. This seems to help
> gcc 4.5 optimizations as well.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/cpu/armv7/omap4/board.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
Pushed to u-boot-ti
Wolfgang Denk - Nov. 19, 2010, 6:37 p.m.
Dear Sandeep,

In message <0554BEF07D437848AF01B9C9B5F0BC5DBD1C0B36@dlee01.ent.ti.com> you wrote:
> 
> Pushed to u-boot-ti

In this case it would make sense to set the state of this patch in
Patchwork to "Awaiting Upstream".  Thanks.

Best regards,

Wolfgang Denk
Sandeep Paulraj - Nov. 19, 2010, 8:04 p.m.
> 
> Dear Sandeep,
> 
> In message <0554BEF07D437848AF01B9C9B5F0BC5DBD1C0B36@dlee01.ent.ti.com>
> you wrote:
> >
> > Pushed to u-boot-ti
> 
> In this case it would make sense to set the state of this patch in
> Patchwork to "Awaiting Upstream".  Thanks.
> 
> Best regards,
> 
> Wolfgang Denk

Wolfgang,

I am working through the todo list on patchworks.

Regards,
Sandeep
Wolfgang Denk - Nov. 19, 2010, 8:18 p.m.
Dear "Paulraj, Sandeep",

In message <0554BEF07D437848AF01B9C9B5F0BC5DBD1C0F84@dlee01.ent.ti.com> you wrote:
> 
> > > Pushed to u-boot-ti
> > 
> > In this case it would make sense to set the state of this patch in
> > Patchwork to "Awaiting Upstream".  Thanks.
...
> I am working through the todo list on patchworks.

Thanks.

Note that this was a "should", not a "must".

Also, there are a few options to do that automatically - either as
post-commit triggers in your git repository, or as a script that
iterates on a list of commits.  See 
http://www.denx.de/wiki/view/U-Boot/Patches#pwclient  and
http://www.denx.de/wiki/view/U-Boot/Patches#pwparser , but be aware
that these scripts may need tuning for your environment (i. e. you
would probably want to change the state to "Awaiting Upstream", and
do this only if the previous state was still "New").

Best regards,

Wolfgang Denk

Patch

diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c
index 24a66f5..f967d09 100644
--- a/arch/arm/cpu/armv7/omap4/board.c
+++ b/arch/arm/cpu/armv7/omap4/board.c
@@ -32,6 +32,8 @@ 
 #include <asm/arch/sys_proto.h>
 #include <asm/sizes.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /*
  * Routine: s_init
  * Description: Does early system init of muxing and clocks.
@@ -100,7 +102,6 @@  u32 sdram_size(void)
  */
 int dram_init(void)
 {
-	DECLARE_GLOBAL_DATA_PTR;
 
 #if defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
 	gd->bd->bi_dram[0].start = 0x80000000;