[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

The -fPIC flag belongs with -mrelocatable, move it there.
Also change -fPIC to -fpic as this produces smaller
binaries.
However, currently -mrelocatable promotes -fpic to -fPIC, a
fix for this is in upcoming gcc 4.6 or you can apply this small
patch to gcc:

diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
index 8da8410..e4b8280 100644
--- a/gcc/config/rs6000/sysv4.h
+++ b/gcc/config/rs6000/sysv4.h
@@ -227,7 +227,8 @@ do {									\
     }									\
 									\
   else if (TARGET_RELOCATABLE)						\
-    flag_pic = 2;							\
+    if (!flag_pic)							\
+      flag_pic = 2;							\
 } while (0)

 #ifndef RS6000_BI_ARCH
--

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/config.mk              |    2 +-
 arch/powerpc/cpu/74xx_7xx/config.mk |    2 +-
 arch/powerpc/cpu/mpc512x/config.mk  |    2 +-
 arch/powerpc/cpu/mpc5xx/config.mk   |    2 +-
 arch/powerpc/cpu/mpc5xxx/config.mk  |    2 +-
 arch/powerpc/cpu/mpc8220/config.mk  |    2 +-
 arch/powerpc/cpu/mpc824x/config.mk  |    2 +-
 arch/powerpc/cpu/mpc8260/config.mk  |    2 +-
 arch/powerpc/cpu/mpc83xx/config.mk  |    2 +-
 arch/powerpc/cpu/mpc85xx/config.mk  |    2 +-
 arch/powerpc/cpu/mpc86xx/config.mk  |    2 +-
 arch/powerpc/cpu/mpc8xx/config.mk   |    2 +-
 arch/powerpc/cpu/ppc4xx/config.mk   |    2 +-
 13 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/config.mk b/arch/powerpc/config.mk
index 2912604..b04e968 100644
--- a/arch/powerpc/config.mk
+++ b/arch/powerpc/config.mk
@@ -25,7 +25,7 @@ CROSS_COMPILE ?= ppc_8xx-
 
 STANDALONE_LOAD_ADDR = 0x40000
 
-PLATFORM_RELFLAGS += -mrelocatable -ffunction-sections -fdata-sections
+PLATFORM_RELFLAGS += -fpic -mrelocatable -ffunction-sections -fdata-sections
 PLATFORM_CPPFLAGS += -DCONFIG_PPC -D__powerpc__
 PLATFORM_LDFLAGS  += -n --gc-sections
 
diff --git a/arch/powerpc/cpu/74xx_7xx/config.mk b/arch/powerpc/cpu/74xx_7xx/config.mk
index df1f6ac..fb0c715 100644
--- a/arch/powerpc/cpu/74xx_7xx/config.mk
+++ b/arch/powerpc/cpu/74xx_7xx/config.mk
@@ -21,6 +21,6 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_74xx_7xx -ffixed-r2 -mstring
diff --git a/arch/powerpc/cpu/mpc512x/config.mk b/arch/powerpc/cpu/mpc512x/config.mk
index baf55cc..8fceded 100644
--- a/arch/powerpc/cpu/mpc512x/config.mk
+++ b/arch/powerpc/cpu/mpc512x/config.mk
@@ -20,7 +20,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC512X -DCONFIG_E300 \
 			-ffixed-r2 -msoft-float -mcpu=603e
diff --git a/arch/powerpc/cpu/mpc5xx/config.mk b/arch/powerpc/cpu/mpc5xx/config.mk
index 1c7df00..ab55040 100644
--- a/arch/powerpc/cpu/mpc5xx/config.mk
+++ b/arch/powerpc/cpu/mpc5xx/config.mk
@@ -21,6 +21,6 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS +=	-fPIC -meabi
+PLATFORM_RELFLAGS +=	-meabi
 
 PLATFORM_CPPFLAGS +=	-DCONFIG_5xx -ffixed-r2 -mpowerpc -msoft-float
diff --git a/arch/powerpc/cpu/mpc5xxx/config.mk b/arch/powerpc/cpu/mpc5xxx/config.mk
index 832909f..fb87980 100644
--- a/arch/powerpc/cpu/mpc5xxx/config.mk
+++ b/arch/powerpc/cpu/mpc5xxx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC5xxx -ffixed-r2 \
 		     -mstring -mcpu=603e -mmultiple
diff --git a/arch/powerpc/cpu/mpc8220/config.mk b/arch/powerpc/cpu/mpc8220/config.mk
index 9142b91..2c638b5 100644
--- a/arch/powerpc/cpu/mpc8220/config.mk
+++ b/arch/powerpc/cpu/mpc8220/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC8220 -ffixed-r2 \
 		     -mstring -mcpu=603e -mmultiple
diff --git a/arch/powerpc/cpu/mpc824x/config.mk b/arch/powerpc/cpu/mpc824x/config.mk
index 65a1771..82b8cc6 100644
--- a/arch/powerpc/cpu/mpc824x/config.mk
+++ b/arch/powerpc/cpu/mpc824x/config.mk
@@ -21,6 +21,6 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC824X -ffixed-r2 -mstring -mcpu=603e -msoft-float
diff --git a/arch/powerpc/cpu/mpc8260/config.mk b/arch/powerpc/cpu/mpc8260/config.mk
index 5e4645a..f091fa9 100644
--- a/arch/powerpc/cpu/mpc8260/config.mk
+++ b/arch/powerpc/cpu/mpc8260/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_8260 -DCONFIG_CPM2 -ffixed-r2 \
 		     -mstring -mcpu=603e -mmultiple
diff --git a/arch/powerpc/cpu/mpc83xx/config.mk b/arch/powerpc/cpu/mpc83xx/config.mk
index 0dced88..a79729e 100644
--- a/arch/powerpc/cpu/mpc83xx/config.mk
+++ b/arch/powerpc/cpu/mpc83xx/config.mk
@@ -20,7 +20,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_MPC83xx -DCONFIG_E300 \
 			-ffixed-r2 -msoft-float
diff --git a/arch/powerpc/cpu/mpc85xx/config.mk b/arch/powerpc/cpu/mpc85xx/config.mk
index 66d1741..68ac57d 100644
--- a/arch/powerpc/cpu/mpc85xx/config.mk
+++ b/arch/powerpc/cpu/mpc85xx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -ffixed-r2 -Wa,-me500 -msoft-float -mno-string
 
diff --git a/arch/powerpc/cpu/mpc86xx/config.mk b/arch/powerpc/cpu/mpc86xx/config.mk
index ca2f837..92ff7bf 100644
--- a/arch/powerpc/cpu/mpc86xx/config.mk
+++ b/arch/powerpc/cpu/mpc86xx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -ffixed-r2 -mstring
 PLATFORM_CPPFLAGS += -maltivec -mabi=altivec -msoft-float
diff --git a/arch/powerpc/cpu/mpc8xx/config.mk b/arch/powerpc/cpu/mpc8xx/config.mk
index f5e08a5..aa61980 100644
--- a/arch/powerpc/cpu/mpc8xx/config.mk
+++ b/arch/powerpc/cpu/mpc8xx/config.mk
@@ -21,6 +21,6 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 
 PLATFORM_CPPFLAGS += -DCONFIG_8xx -ffixed-r2 -mstring -mcpu=860 -msoft-float
diff --git a/arch/powerpc/cpu/ppc4xx/config.mk b/arch/powerpc/cpu/ppc4xx/config.mk
index d862bb4..f5cbbbd 100644
--- a/arch/powerpc/cpu/ppc4xx/config.mk
+++ b/arch/powerpc/cpu/ppc4xx/config.mk
@@ -21,7 +21,7 @@
 # MA 02111-1307 USA
 #
 
-PLATFORM_RELFLAGS += -fPIC -meabi
+PLATFORM_RELFLAGS += -meabi
 PLATFORM_CPPFLAGS += -DCONFIG_4xx -ffixed-r2 -mstring -msoft-float
 
 cfg=$(shell grep configs $(OBJTREE)/include/config.h | sed 's/.*<\(configs.*\)>/\1/')
-- 
1.7.2.2

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1291642606-4290-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> The -fPIC flag belongs with -mrelocatable, move it there.
> Also change -fPIC to -fpic as this produces smaller
> binaries.
> However, currently -mrelocatable promotes -fpic to -fPIC, a
> fix for this is in upcoming gcc 4.6 or you can apply this small
> patch to gcc:
> 
> diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
> index 8da8410..e4b8280 100644
> --- a/gcc/config/rs6000/sysv4.h
> +++ b/gcc/config/rs6000/sysv4.h
> @@ -227,7 +227,8 @@ do {									\
>      }									\
>  									\
>    else if (TARGET_RELOCATABLE)						\
> -    flag_pic = 2;							\
> +    if (!flag_pic)							\
> +      flag_pic = 2;							\
>  } while (0)
> 
>  #ifndef RS6000_BI_ARCH
> --
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

Applied, thanks.

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: wd at denx.de
"Intelligence without character is a dangerous thing."   - G. Steinem

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1291642606-4290-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> The -fPIC flag belongs with -mrelocatable, move it there.
> Also change -fPIC to -fpic as this produces smaller
> binaries.
> However, currently -mrelocatable promotes -fpic to -fPIC, a
> fix for this is in upcoming gcc 4.6 or you can apply this small
> patch to gcc:

Unfortunately this patch breaks a number of boards:

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[33ee4c92339ee386662c0ee2d221098c5cc8b07e] PowerPC: Move -fPIC flag to
common place


They fail to build like this:

...
Configuring for TQM855L board...
/work/tmp-8xx/u-boot.lds:74 cannot move location counter backwards (from 4000800c to 40008000)
make: *** [/work/tmp-8xx/u-boot] Error 1
ppc_6xx-size: '/work/tmp-8xx/u-boot': No such file
...
Configuring for TQM860L board...
/work/tmp-8xx/u-boot.lds:74 cannot move location counter backwards (from 4000802c to 40008000)
make: *** [/work/tmp-8xx/u-boot] Error 1
ppc_6xx-size: '/work/tmp-8xx/u-boot': No such file

It seems this change increases the code size somehow - which I would
not expect from your comment above.  Please have a look.

Thanks.

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: wd at denx.de
Military secrets are the most fleeting of all.
	-- Spock, "The Enterprise Incident", stardate 5027.4

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

>
> Dear Joakim Tjernlund,
>
> In message <1291642606-4290-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > The -fPIC flag belongs with -mrelocatable, move it there.
> > Also change -fPIC to -fpic as this produces smaller
> > binaries.
> > However, currently -mrelocatable promotes -fpic to -fPIC, a
> > fix for this is in upcoming gcc 4.6 or you can apply this small
> > patch to gcc:
>
> Unfortunately this patch breaks a number of boards:
>
> Bisecting: 0 revisions left to test after this (roughly 0 steps)
> [33ee4c92339ee386662c0ee2d221098c5cc8b07e] PowerPC: Move -fPIC flag to
> common place
>
>
> They fail to build like this:
>
> ...
> Configuring for TQM855L board...
> /work/tmp-8xx/u-boot.lds:74 cannot move location counter backwards (from 4000800c to 40008000)
> make: *** [/work/tmp-8xx/u-boot] Error 1
> ppc_6xx-size: '/work/tmp-8xx/u-boot': No such file
> ...
> Configuring for TQM860L board...
> /work/tmp-8xx/u-boot.lds:74 cannot move location counter backwards (from 4000802c to 40008000)
> make: *** [/work/tmp-8xx/u-boot] Error 1
> ppc_6xx-size: '/work/tmp-8xx/u-boot': No such file
>
> It seems this change increases the code size somehow - which I would
> not expect from your comment above.  Please have a look.

Very strange, I had a quick look(that is all I can do ATM). It appers
that 8xx is the only ppc arch which doesn't have its own u-boot.lds under
arch/powerpc/cpu/mpc8xx, is that intentional? Is it only 8xx that has this
problem?
My guess would be that 8xx missed out on an earlier patch
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=337f5f50f539cc1ea1e0533c096e237228f12cae

 Jocke

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF484EFF4D.45D9734B-ONC1257870.002E25B2-C1257870.002F0BE0@transmode.se> you wrote:
>
> > It seems this change increases the code size somehow - which I would
> > not expect from your comment above.  Please have a look.
> 
> Very strange, I had a quick look(that is all I can do ATM). It appers
> that 8xx is the only ppc arch which doesn't have its own u-boot.lds under
> arch/powerpc/cpu/mpc8xx, is that intentional? Is it only 8xx that has this
> problem?

I did not look closer, but it appears that these boards are all boards
that use an embedded environment, and thus their own, usually manually
tuned linker script.

> My guess would be that 8xx missed out on an earlier patch
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=337f5f50f539cc1ea1e0533c096e237228f12cae

Or, more correctly, that this patch failed to patch these boards.

Can you please provide a fix?  I'd like to get rid of this build error
ASAP.  We could, of course, simply revert these commits.

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: wd at denx.de
If God had a beard, he'd be a UNIX programmer.

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/19 22:25:25:
>
> Dear Joakim Tjernlund,
>
> In message <OF484EFF4D.45D9734B-ONC1257870.002E25B2-C1257870.002F0BE0@transmode.se> you wrote:
> >
> > > It seems this change increases the code size somehow - which I would
> > > not expect from your comment above.  Please have a look.
> >
> > Very strange, I had a quick look(that is all I can do ATM). It appers
> > that 8xx is the only ppc arch which doesn't have its own u-boot.lds under
> > arch/powerpc/cpu/mpc8xx, is that intentional? Is it only 8xx that has this
> > problem?
>
> I did not look closer, but it appears that these boards are all boards
> that use an embedded environment, and thus their own, usually manually
> tuned linker script.
>
> > My guess would be that 8xx missed out on an earlier patch
> > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=337f5f50f539cc1ea1e0533c096e237228f12cae
>
> Or, more correctly, that this patch failed to patch these boards.
>
> Can you please provide a fix?  I'd like to get rid of this build error
> ASAP.  We could, of course, simply revert these commits.

Grepping shows a boat load of u-boot.lds files under board.  I don't
have time to patch them all ATM. They really needs to be consolidated
somehow but that is over my head.

  Jocke

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFC3D6185B.85DCE339-ONC1257877.007406B0-C1257877.00746A1D@transmode.se> you wrote:
>
> > Can you please provide a fix?  I'd like to get rid of this build error
> > ASAP.  We could, of course, simply revert these commits.
> 
> Grepping shows a boat load of u-boot.lds files under board.  I don't
> have time to patch them all ATM. They really needs to be consolidated
> somehow but that is over my head.

That means I will revert the two commits?

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: wd at denx.de
In the future, you're going to get computers as prizes  in  breakfast
cereals.  You'll  throw  them out because your house will be littered
with them.                                             - Robert Lucky

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/19 23:35:40:
>
> Dear Joakim Tjernlund,
>
> In message <OFC3D6185B.85DCE339-ONC1257877.007406B0-C1257877.00746A1D@transmode.se> you wrote:
> >
> > > Can you please provide a fix?  I'd like to get rid of this build error
> > > ASAP.  We could, of course, simply revert these commits.
> >
> > Grepping shows a boat load of u-boot.lds files under board.  I don't
> > have time to patch them all ATM. They really needs to be consolidated
> > somehow but that is over my head.
>
> That means I will revert the two commits?

One, you only claimed that:
> The -fPIC flag belongs with -mrelocatable, move it there.
> Also change -fPIC to -fpic as this produces smaller
> binaries.
> However, currently -mrelocatable promotes -fpic to -fPIC, a
> fix for this is in upcoming gcc 4.6 or you can apply this small
> patch to gcc:
Broke all those 8xx boards.

  Jocke

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFC5A52DF3.355573B0-ONC1257877.0078430F-C1257877.0078910D@transmode.se> you wrote:
>
> > That means I will revert the two commits?
> 
> One, you only claimed that:
> > The -fPIC flag belongs with -mrelocatable, move it there.
> > Also change -fPIC to -fpic as this produces smaller
> > binaries.
> > However, currently -mrelocatable promotes -fpic to -fPIC, a
> > fix for this is in upcoming gcc 4.6 or you can apply this small
> > patch to gcc:
> Broke all those 8xx boards.

Yes, but you yorself pointed out that commit 337f5f5 missed a large
number of boards, leaving the tree in a inconsistent state. Should we
not revert that one as well?

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: wd at denx.de
Alles Gescheite ist schon gedacht worden, man mu? nur versuchen,
es noch einmal zu denken.          -- Goethe, Maximen und Reflexionen

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/20 00:05:27:

> From: Wolfgang Denk <wd@denx.de>
> To: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Cc: u-boot at lists.denx.de
> Date: 2011/04/20 00:05
> Subject: Re: [U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place
>
> Dear Joakim Tjernlund,
>
> In message <OFC5A52DF3.355573B0-ONC1257877.0078430F-C1257877.0078910D@transmode.se> you wrote:
> >
> > > That means I will revert the two commits?
> >
> > One, you only claimed that:
> > > The -fPIC flag belongs with -mrelocatable, move it there.
> > > Also change -fPIC to -fpic as this produces smaller
> > > binaries.
> > > However, currently -mrelocatable promotes -fpic to -fPIC, a
> > > fix for this is in upcoming gcc 4.6 or you can apply this small
> > > patch to gcc:
> > Broke all those 8xx boards.
>
> Yes, but you yorself pointed out that commit 337f5f5 missed a large
> number of boards, leaving the tree in a inconsistent state. Should we
> not revert that one as well?

It is not too bad, just not complete yet. Reverting all just makes
it harder to get it all done.
I still don't get why it broke really.
hmm, did you by any chance include my patches to gcc too?
if so I you should only have to fixup arch/powerpc/config.mk:
- PLATFORM_RELFLAGS += -fpic -mrelocatable -ffunction-sections -fdata-sections
- PLATFORM_RELFLAGS += $(call cc-option,-msingle-pic-base,)
+ PLATFORM_RELFLAGS += -fPIC -mrelocatable -ffunction-sections -fdata-sections

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFBC9C03BC.436C27C7-ONC1257877.00797190-C1257877.007A0FD7@transmode.se> you wrote:
> 
> > Yes, but you yorself pointed out that commit 337f5f5 missed a large
> > number of boards, leaving the tree in a inconsistent state. Should we
> > not revert that one as well?
> 
> It is not too bad, just not complete yet. Reverting all just makes
> it harder to get it all done.
> I still don't get why it broke really.
> hmm, did you by any chance include my patches to gcc too?
> if so I you should only have to fixup arch/powerpc/config.mk:
> - PLATFORM_RELFLAGS += -fpic -mrelocatable -ffunction-sections -fdata-sections
> - PLATFORM_RELFLAGS += $(call cc-option,-msingle-pic-base,)
> + PLATFORM_RELFLAGS += -fPIC -mrelocatable -ffunction-sections -fdata-sections

Can you please have a look at this yourslef?  You understand much
better than me which of your patches are doing what, and how they
might interact.

I don't have the time to dig into this any deeper.  If we cannot find
a quick solution, I suggest to back out all this stuff and redo once
the problems have been sorted out.  Sorry for applying the stuff to
mainline without more thorough testing, I should have noticed these
issues earlier and never checked in all that stuff.

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: wd at denx.de
Another megabytes the dust.

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/20 00:41:01:
>
> Dear Joakim Tjernlund,
>
> In message <OFBC9C03BC.436C27C7-ONC1257877.00797190-C1257877.007A0FD7@transmode.se> you wrote:
> >
> > > Yes, but you yorself pointed out that commit 337f5f5 missed a large
> > > number of boards, leaving the tree in a inconsistent state. Should we
> > > not revert that one as well?
> >
> > It is not too bad, just not complete yet. Reverting all just makes
> > it harder to get it all done.
> > I still don't get why it broke really.
> > hmm, did you by any chance include my patches to gcc too?
> > if so I you should only have to fixup arch/powerpc/config.mk:
> > - PLATFORM_RELFLAGS += -fpic -mrelocatable -ffunction-sections -fdata-sections
> > - PLATFORM_RELFLAGS += $(call cc-option,-msingle-pic-base,)
> > + PLATFORM_RELFLAGS += -fPIC -mrelocatable -ffunction-sections -fdata-sections
>
> Can you please have a look at this yourslef?  You understand much
> better than me which of your patches are doing what, and how they
> might interact.

Tell me this first:
 "hmm, did you by any chance include my patches to gcc too?"
and I might have a fix for you. tmw, need to sleep now

>
> I don't have the time to dig into this any deeper.  If we cannot find
> a quick solution, I suggest to back out all this stuff and redo once
> the problems have been sorted out.  Sorry for applying the stuff to
> mainline without more thorough testing, I should have noticed these
> issues earlier and never checked in all that stuff.
>
> 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: wd at denx.de
> Another megabytes the dust.

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/20 00:41:01:
>
> Dear Joakim Tjernlund,
>
> In message <OFBC9C03BC.436C27C7-ONC1257877.00797190-C1257877.007A0FD7@transmode.se> you wrote:
> >
> > > Yes, but you yorself pointed out that commit 337f5f5 missed a large
> > > number of boards, leaving the tree in a inconsistent state. Should we
> > > not revert that one as well?
> >
> > It is not too bad, just not complete yet. Reverting all just makes
> > it harder to get it all done.
> > I still don't get why it broke really.
> > hmm, did you by any chance include my patches to gcc too?
> > if so I you should only have to fixup arch/powerpc/config.mk:
> > - PLATFORM_RELFLAGS += -fpic -mrelocatable -ffunction-sections -fdata-sections
> > - PLATFORM_RELFLAGS += $(call cc-option,-msingle-pic-base,)
> > + PLATFORM_RELFLAGS += -fPIC -mrelocatable -ffunction-sections -fdata-sections
>
> Can you please have a look at this yourslef?  You understand much
> better than me which of your patches are doing what, and how they
> might interact.
>
> I don't have the time to dig into this any deeper.  If we cannot find
> a quick solution, I suggest to back out all this stuff and redo once
> the problems have been sorted out.  Sorry for applying the stuff to
> mainline without more thorough testing, I should have noticed these
> issues earlier and never checked in all that stuff.

OK, I managed to script the change, patch last in mail.

However CROSS_COMPILE=powerpc-softfloat-linux-gnu- ./MAKEALL  TQM862L TQM855L TQM860L
does not build for me, same problem as you got.
Even if I manually back out the 2 patches affecting 8xx it wont build.
I think these boards have some other problem really, like u-boot is too big for them.

Most other 8xx boards build fine, those that don't have some other problem.

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF8B32E14F.150B51B9-ONC1257878.0044927E-C1257878.00452684@transmode.se> you wrote:
>
> OK, I managed to script the change, patch last in mail.

Thanks.

> However CROSS_COMPILE=powerpc-softfloat-linux-gnu- ./MAKEALL  TQM862L TQM855L TQM860L
> does not build for me, same problem as you got.
> Even if I manually back out the 2 patches affecting 8xx it wont build.
> I think these boards have some other problem really, like u-boot is too big for them.
> 
> Most other 8xx boards build fine, those that don't have some other problem.

I bisected the build issue, and it pointed at your patch.


> From ce7970dd177a798c5ab7e093adadfc57a97ce0e7 Mon Sep 17 00:00:00 2001
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Wed, 20 Apr 2011 14:22:59 +0200
> Subject: [PATCH] powerpc, 8xx: Fixup all 8xx u-boot.lds scripts
> 
> 8xx was left behind when fixing up powerpc linking
> scripts to support -fpic.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  board/LEOX/elpt860/u-boot.lds        |    5 +++--
>  board/RPXClassic/u-boot.lds          |    5 +++--
>  board/RPXlite/u-boot.lds             |    5 +++--
>  board/RPXlite_dw/u-boot.lds          |    5 +++--
>  board/RRvision/u-boot.lds            |    5 +++--
>  board/adder/u-boot.lds               |    5 +++--
>  board/amirix/ap1000/u-boot.lds       |    5 +++--
>  board/c2mon/u-boot.lds               |    5 +++--
>  board/cogent/u-boot.lds              |    5 +++--
>  board/dave/PPChameleonEVB/u-boot.lds |    3 ++-
>  board/eltec/mhpc/u-boot.lds          |    5 +++--
>  board/emk/top860/u-boot.lds          |    5 +++--
>  board/ep88x/u-boot.lds               |    3 ++-
>  board/esd/dasa_sim/u-boot.lds        |    5 +++--
>  board/esteem192e/u-boot.lds          |    5 +++--
>  board/etx094/u-boot.lds              |    5 +++--
>  board/evb64260/u-boot.lds            |    5 +++--
>  board/fads/u-boot.lds                |    5 +++--
>  board/flagadm/u-boot.lds             |    5 +++--
>  board/gen860t/u-boot.lds             |    5 +++--
>  board/genietv/u-boot.lds             |    5 +++--
>  board/hermes/u-boot.lds              |    5 +++--
>  board/hymod/u-boot.lds               |    2 +-
>  board/icu862/u-boot.lds              |    5 +++--
>  board/ip860/u-boot.lds               |    5 +++--
>  board/ivm/u-boot.lds                 |    5 +++--
>  board/kup/kup4k/u-boot.lds           |    5 +++--
>  board/kup/kup4x/u-boot.lds           |    5 +++--
>  board/lantec/u-boot.lds              |    5 +++--
>  board/lwmon/u-boot.lds               |    5 +++--
>  board/manroland/uc100/u-boot.lds     |    5 +++--
>  board/matrix_vision/mvsmr/u-boot.lds |    3 ++-
>  board/mbx8xx/u-boot.lds              |    5 +++--
>  board/ml2/u-boot.lds                 |    5 +++--
>  board/mousse/u-boot.lds              |    5 +++--
>  board/mvblue/u-boot.lds              |    5 +++--
>  board/netphone/u-boot.lds            |    5 +++--
>  board/netta/u-boot.lds               |    5 +++--
>  board/netta2/u-boot.lds              |    5 +++--
>  board/netvia/u-boot.lds              |    5 +++--
>  board/nx823/u-boot.lds               |    5 +++--
>  board/quantum/u-boot.lds             |    5 +++--
>  board/r360mpi/u-boot.lds             |    5 +++--
>  board/rbc823/u-boot.lds              |    5 +++--
>  board/rmu/u-boot.lds                 |    5 +++--
>  board/rsdproto/u-boot.lds            |    2 +-
>  board/sandpoint/u-boot.lds           |    5 +++--
>  board/sc3/u-boot.lds                 |    5 +++--
>  board/siemens/IAD210/u-boot.lds      |    5 +++--
>  board/sixnet/u-boot.lds              |    5 +++--
>  board/snmc/qs850/u-boot.lds          |    5 +++--
>  board/snmc/qs860t/u-boot.lds         |    5 +++--
>  board/spc1920/u-boot.lds             |    5 +++--
>  board/spd8xx/u-boot.lds              |    5 +++--
>  board/stx/stxxtc/u-boot.lds          |    5 +++--
>  board/svm_sc8xx/u-boot.lds           |    5 +++--
>  board/tqc/tqm8xx/u-boot.lds          |    5 +++--
>  board/v37/u-boot.lds                 |    5 +++--
>  board/westel/amx860/u-boot.lds       |    5 +++--
>  59 files changed, 170 insertions(+), 113 deletions(-)

Thanks, applied.

Problem is still present.

Bisected again:

39768f7715ed637ef02f49fc7de664cc1aaf14b3 is the first bad commit
commit 39768f7715ed637ef02f49fc7de664cc1aaf14b3
Author: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date:   Mon Dec 6 18:35:37 2010 +0100

    PowerPC: Add support for -msingle-pic-base


I revert this commit now.  Result:

-> ./MAKEALL TQM860L
Configuring for TQM860L board...
u-boot.lds:75 cannot move location counter backwards (from 40008024 to 40008000)
make: *** [u-boot] Error 1
ppc_8xx-size: './u-boot': No such file

--------------------- SUMMARY ----------------------------
Boards compiled: 1
Boards with warnings or errors: 1 ( TQM860L )
----------------------------------------------------------
-> git revert 39768f7715ed637ef02f49fc7de664cc1aaf14b3
[master 8c4734e] Revert "PowerPC: Add support for -msingle-pic-base"
 13 files changed, 12 insertions(+), 127 deletions(-)
-> ./MAKEALL TQM860L
Configuring for TQM860L board...
   text    data     bss     dec     hex filename
 258127   14768   27228  300123   4945b ./u-boot

--------------------- SUMMARY ----------------------------
Boards compiled: 1
----------------------------------------------------------


Please feel free to resubmit when this commit does not cause problems
any more.

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: wd at denx.de
Real Programmers always confuse Christmas and Halloween because
OCT 31 == DEC 25 !  - Andrew Rutherford (andrewr at ucs.adelaide.edu.au)

[U-Boot] [PATCH] PowerPC: Move -fPIC flag to common place

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/20 22:13:35:
>
> Dear Joakim Tjernlund,
>
> In message <OF8B32E14F.150B51B9-ONC1257878.0044927E-C1257878.00452684@transmode.se> you wrote:
> >
> > OK, I managed to script the change, patch last in mail.
>
> Thanks.
>
> > However CROSS_COMPILE=powerpc-softfloat-linux-gnu- ./MAKEALL  TQM862L TQM855L TQM860L
> > does not build for me, same problem as you got.
> > Even if I manually back out the 2 patches affecting 8xx it wont build.
> > I think these boards have some other problem really, like u-boot is too big for them.
> >
> > Most other 8xx boards build fine, those that don't have some other problem.
>
> I bisected the build issue, and it pointed at your patch.

hmm, this is strange. The only way I managed to build these boards
was with my patches applied and with a gcc that understands -msingle-pic-base et. all

Backing out all 3 patches did not help. Not with gcc 3.4.6, 4.4.5 or 4.5.2
so I am really lost why your gcc(of which I know nothing) works.

I suspect it is just plain luck that it works for you. If you add
-fno-jump-tables to your gcc, does it break?

 Jocke

[U-Boot] [PATCH 0/3] Make 8xx build with -msingle-pic-base

[Joakim Tjernlund]

With these patches 8xx willbuild again for me
with a number of gcc's even with "Add support for -msingle-pic-base"
I hope WD's gcc will also work.

Joakim Tjernlund (3):
  powerpc, 8xx: Fix fallout from "Fixup all 8xx u-boot.lds scripts"
  Fix a few gcc warnings.
  powerpc, 8xx: adapt a few linker scripts so it will build.

 board/LEOX/elpt860/u-boot.lds        |    2 +-
 board/etx094/u-boot.lds              |    2 +-
 board/matrix_vision/mvsmr/u-boot.lds |    2 +-
 board/netta/codec.c                  |    6 +++---
 board/rsdproto/u-boot.lds            |    7 ++++---
 board/siemens/IAD210/atm.c           |    4 ++--
 board/svm_sc8xx/u-boot.lds           |    2 +-
 board/tqc/tqm8xx/u-boot.lds          |    2 ++
 examples/standalone/timer.c          |    4 ++--
 9 files changed, 17 insertions(+), 14 deletions(-)

-- 
1.7.3.4

[U-Boot] [PATCH 1/3] powerpc, 8xx: Fix fallout from "Fixup all 8xx u-boot.lds scripts"

[Joakim Tjernlund]

Two linker scripts for 8xx was missed.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 board/matrix_vision/mvsmr/u-boot.lds |    2 +-
 board/rsdproto/u-boot.lds            |    7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/board/matrix_vision/mvsmr/u-boot.lds b/board/matrix_vision/mvsmr/u-boot.lds
index bf2ed04..57c37de 100644
--- a/board/matrix_vision/mvsmr/u-boot.lds
+++ b/board/matrix_vision/mvsmr/u-boot.lds
@@ -62,7 +62,7 @@ SECTIONS
     _FIXUP_TABLE_ = .;
     KEEP(*(.fixup))
   }
-  __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2;
+  __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
   __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
 
   .data    :
diff --git a/board/rsdproto/u-boot.lds b/board/rsdproto/u-boot.lds
index 81728db..a729c52 100644
--- a/board/rsdproto/u-boot.lds
+++ b/board/rsdproto/u-boot.lds
@@ -74,11 +74,12 @@ SECTIONS
   PROVIDE (erotext = .);
   .reloc   :
   {
-    *(.got)
     _GOT2_TABLE_ = .;
-    *(.got2)
+    KEEP(*(.got2))
+    KEEP(*(.got))
+    PROVIDE(_GLOBAL_OFFSET_TABLE_ = . + 4);
     _FIXUP_TABLE_ = .;
-    *(.fixup)
+    KEEP(*(.fixup))
   }
   __got2_entries = ((_GLOBAL_OFFSET_TABLE_ - _GOT2_TABLE_) >> 2) - 1;
   __fixup_entries = (. - _FIXUP_TABLE_)>>2;
-- 
1.7.3.4

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Joakim Tjernlund]

Noticed while building all of mpc8xx

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 board/netta/codec.c         |    6 +++---
 board/siemens/IAD210/atm.c  |    4 ++--
 examples/standalone/timer.c |    4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/board/netta/codec.c b/board/netta/codec.c
index 844aa18..c8d31d7 100644
--- a/board/netta/codec.c
+++ b/board/netta/codec.c
@@ -502,7 +502,7 @@ void codsp_write_sop_short(int duslic_id, int channel, unsigned char regno, unsi
 
 void codsp_write_sop_int(int duslic_id, int channel, unsigned char regno, unsigned int val)
 {
-	unsigned char cmd[5];
+	unsigned char cmd[6];
 
 	cmd[0] = CODSP_WR | CODSP_ADR(channel) | CODSP_CMD_SOP;
 	cmd[1] = regno;
@@ -577,7 +577,7 @@ void codsp_write_cop_char(int duslic_id, int channel, unsigned char addr, unsign
 
 void codsp_write_cop_short(int duslic_id, int channel, unsigned char addr, unsigned short val)
 {
-	unsigned char cmd[3];
+	unsigned char cmd[4];
 
 	cmd[0] = CODSP_WR | CODSP_OP | CODSP_ADR(channel) | CODSP_CMD_COP;
 	cmd[1] = addr;
@@ -668,7 +668,7 @@ void codsp_write_pop_short (int duslic_id, int channel, unsigned char regno,
 void codsp_write_pop_int (int duslic_id, int channel, unsigned char regno,
 			  unsigned int val)
 {
-	unsigned char cmd[5];
+	unsigned char cmd[6];
 
 	cmd[0] = CODSP_WR | CODSP_ADR (channel) | CODSP_CMD_POP;
 	cmd[1] = regno;
diff --git a/board/siemens/IAD210/atm.c b/board/siemens/IAD210/atm.c
index e599c10..40aad0a 100644
--- a/board/siemens/IAD210/atm.c
+++ b/board/siemens/IAD210/atm.c
@@ -62,7 +62,7 @@ int atmLoad()
   volatile iop8xx_t      *iop    = &immap->im_ioport;
 
   timers->cpmt_tgcr &=  0x0FFF; SYNC;             /* Disable Timer 4 */
-  immap->im_cpm.cp_scc[4].scc_gsmrl = 0x0; SYNC; /* Disable SCC4 */
+  immap->im_cpm.cp_scc[3].scc_gsmrl = 0x0; SYNC; /* Disable SCC4 */
   iop->iop_pdpar &= 0x3FFF; SYNC;                 /* Disable SAR and UTOPIA */
 
   if ( atmMemInit() != OK ) return ERROR;
@@ -96,7 +96,7 @@ void atmUnload()
   volatile iop8xx_t      *iop    = &immap->im_ioport;
 
   timers->cpmt_tgcr &=  0x0FFF; SYNC;             /* Disable Timer 4 */
-  immap->im_cpm.cp_scc[4].scc_gsmrl = 0x0; SYNC;  /* Disable SCC4 */
+  immap->im_cpm.cp_scc[3].scc_gsmrl = 0x0; SYNC;  /* Disable SCC4 */
   iop->iop_pdpar &= 0x3FFF; SYNC;                 /* Disable SAR and UTOPIA */
   g_atm.loaded = FALSE;
 }
diff --git a/examples/standalone/timer.c b/examples/standalone/timer.c
index 834cc9a..a4d4581 100644
--- a/examples/standalone/timer.c
+++ b/examples/standalone/timer.c
@@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
 	/* clear all events */
 	*hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
 
-	printf (usage);
+	printf("%s", usage);
 	running = 0;
 	while ((c = getc()) != 'q') {
 	    if (c == 'b') {
@@ -255,7 +255,7 @@ int timer (int argc, char * const argv[])
 	    } else {
 		printf ("\nEnter: q - quit, b - start timer, e - stop timer, ? - get status\n");
 	    }
-	    printf (usage);
+	    printf("%s", usage);
 	}
 	if (running) {
 		printf ("Stopping timer\n");
-- 
1.7.3.4

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Joakim Tjernlund]

Seems like recent linker/fpic changes broke a few 8xx boards.
This makes them build again.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 board/LEOX/elpt860/u-boot.lds |    2 +-
 board/etx094/u-boot.lds       |    2 +-
 board/svm_sc8xx/u-boot.lds    |    2 +-
 board/tqc/tqm8xx/u-boot.lds   |    2 ++
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/board/LEOX/elpt860/u-boot.lds b/board/LEOX/elpt860/u-boot.lds
index 5aaf6b3..37b74ec 100644
--- a/board/LEOX/elpt860/u-boot.lds
+++ b/board/LEOX/elpt860/u-boot.lds
@@ -43,7 +43,7 @@ SECTIONS
 
     arch/powerpc/cpu/mpc8xx/start.o	(.text*)
     arch/powerpc/cpu/mpc8xx/traps.o	(.text*)
-    common/libcommon.o			(.text*)
+    /* common/libcommon.o			(.text*) */
     arch/powerpc/cpu/mpc8xx/libmpc8xx.o	(.text*)
     board/LEOX/elpt860/libelpt860.o	(.text*)
     arch/powerpc/lib/libpowerpc.o	(.text*)
diff --git a/board/etx094/u-boot.lds b/board/etx094/u-boot.lds
index b68d9ea..33c4dba 100644
--- a/board/etx094/u-boot.lds
+++ b/board/etx094/u-boot.lds
@@ -36,7 +36,7 @@ SECTIONS
     arch/powerpc/cpu/mpc8xx/traps.o	(.text*)
     net/libnet.o			(.text*)
     arch/powerpc/cpu/mpc8xx/libmpc8xx.o	(.text*)
-    *(.text.*printf*)
+    /* *(.text.*printf*) */
 
     . = env_offset;
     common/env_embedded.o		(.text*)
diff --git a/board/svm_sc8xx/u-boot.lds b/board/svm_sc8xx/u-boot.lds
index c65f022..7b886f0 100644
--- a/board/svm_sc8xx/u-boot.lds
+++ b/board/svm_sc8xx/u-boot.lds
@@ -41,7 +41,7 @@ SECTIONS
     *(.text.*printf)
     *(.text.do_mem_*)
     *(.text.flash*)
-    *(.text.run_command)
+/*    *(.text.run_command) */
     *(.text.main_loop)
     *(.text.srec_decode)
 
diff --git a/board/tqc/tqm8xx/u-boot.lds b/board/tqc/tqm8xx/u-boot.lds
index f625c3d..43bfe1c 100644
--- a/board/tqc/tqm8xx/u-boot.lds
+++ b/board/tqc/tqm8xx/u-boot.lds
@@ -42,8 +42,10 @@ SECTIONS
     drivers/pcmcia/libpcmcia.o		(.text*)
     drivers/rtc/librtc.o		(.text*)
     drivers/misc/libmisc.o		(.text*)
+/*
     *(.text.print_buffer)
     *(.text.print_size)
+*/
 
     . = DEFINED(env_offset) ? env_offset : .;
     common/env_embedded.o	(.ppcenv*)
-- 
1.7.3.4

[U-Boot] [PATCH 0/3] Make 8xx build with -msingle-pic-base

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1303633774-22961-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> With these patches 8xx willbuild again for me
> with a number of gcc's even with "Add support for -msingle-pic-base"
> I hope WD's gcc will also work.
> 
> Joakim Tjernlund (3):
>   powerpc, 8xx: Fix fallout from "Fixup all 8xx u-boot.lds scripts"
>   Fix a few gcc warnings.
>   powerpc, 8xx: adapt a few linker scripts so it will build.

What do you mean with "will build again"?

You are aware that the problemativ patch has been reverted, aren't
you?

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: wd at denx.de
Documentation is the castor oil of programming.
Managers know it must be good because the programmers hate it so much.

[U-Boot] [PATCH 0/3] Make 8xx build with -msingle-pic-base

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/24 16:30:14:
>
> Dear Joakim Tjernlund,
>
> In message <1303633774-22961-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > With these patches 8xx willbuild again for me
> > with a number of gcc's even with "Add support for -msingle-pic-base"
> > I hope WD's gcc will also work.
> >
> > Joakim Tjernlund (3):
> >   powerpc, 8xx: Fix fallout from "Fixup all 8xx u-boot.lds scripts"
> >   Fix a few gcc warnings.
> >   powerpc, 8xx: adapt a few linker scripts so it will build.
>
> What do you mean with "will build again"?
>
> You are aware that the problemativ patch has been reverted, aren't
> you?

Yes, but I have it here locally. With it and any compiler I have handy, the problematic
board builds OK. It just needs to pass your gcc as well :)
Probably you could just test these new patches as is and if that works well, could you
try "unreverting" the problematic patch again?

 Jocke

[U-Boot] [PATCH 0/3] Make 8xx build with -msingle-pic-base

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFCB7A24F0.EBD815BE-ONC125787C.005061DD-C125787C.0050B36F@transmode.se> you wrote:
>
> Yes, but I have it here locally. With it and any compiler I have handy, the problematic
> board builds OK. It just needs to pass your gcc as well :)
> Probably you could just test these new patches as is and if that works well, could you
> try "unreverting" the problematic patch again?

I can try for test purposes, but if it works, I would rather like to
see a single patch that implkements this correctly in a single commit
so we maintain bisectability.

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: wd at denx.de
"You can have my Unix system when you  pry  it  from  my  cold,  dead
fingers."                                                - Cal Keegan

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1303633774-22961-4-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Seems like recent linker/fpic changes broke a few 8xx boards.
> This makes them build again.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  board/LEOX/elpt860/u-boot.lds |    2 +-
>  board/etx094/u-boot.lds       |    2 +-
>  board/svm_sc8xx/u-boot.lds    |    2 +-
>  board/tqc/tqm8xx/u-boot.lds   |    2 ++
>  4 files changed, 5 insertions(+), 3 deletions(-)

Sorry, but isn't this just papering over the original problem?
In the reverted commit you claimed:

    -msingle-pic-base is a new gcc option for ppc and
    it reduces the size of my u-boot with 6-8 KB.
    While at it, add -fno-jump-tables too to save a
    few more bytes.

This promises to reduce the overall code size, but instead the
observation is that it is growing, which is the reson for the build
problem on some boards.

For example TQM860L (top of tree, with 8c4734e reverted (i. e. your
old 39768f7 reapplied) and this patch here applied, I see:

   text    data     bss     dec     hex filename
 258663   14676   27228  300567   49617 ./u-boot

and

                0x00008000                . = DEFINED (env_offset)?env_offset:.
 *fill*         0x40007d28      0x2d8 00
 common/env_embedded.o(.ppcenv*)
 .ppcenv        0x40008000     0x8000 common/env_embedded.o

Without these two changes we have:

   text    data     bss     dec     hex filename
 257955   14744   27228  299927   49397 ./u-boot

and

 670                 0x00008000                . = DEFINED (env_offset)?env_offset:.
 671  *fill*         0x40007fb4       0x4c 00
 672  common/env_embedded.o(.ppcenv*)
 673  .ppcenv        0x40008000     0x8000 common/env_embedded.o


The gap before the embedded environment which is just 76 bytes
without your patches now grows to 728 bytes (i. e. by a factor of
nearly 10), and the total image sie grows, too.


I will not apply either of these commits - neither the reverted one,
nor this additional one, as both of them grow the memory footprint
instead of redusing it as you promised in the commit message.

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: wd at denx.de
The complexity of software is an essential property, not an  acciden-
tal  one. Hence, descriptions of a software entity that abstract away
its complexity often abstract away its essence.    - Fred Brooks, Jr.

[U-Boot] [PATCH 1/3] powerpc, 8xx: Fix fallout from "Fixup all 8xx u-boot.lds scripts"

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1303633774-22961-2-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Two linker scripts for 8xx was missed.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  board/matrix_vision/mvsmr/u-boot.lds |    2 +-
>  board/rsdproto/u-boot.lds            |    7 ++++---
>  2 files changed, 5 insertions(+), 4 deletions(-)

Applied, thanks.

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: wd at denx.de
Our OS who art in CPU, UNIX be thy name.
Thy programs run, thy syscalls done,
In kernel as it is in user!

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1303633774-22961-3-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Noticed while building all of mpc8xx

Please include in the commit message what the compiler warnings were,
and which compiler version was used to produce these warnings.


> diff --git a/examples/standalone/timer.c b/examples/standalone/timer.c
> index 834cc9a..a4d4581 100644
> --- a/examples/standalone/timer.c
> +++ b/examples/standalone/timer.c
> @@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
>  	/* clear all events */
>  	*hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
>  
> -	printf (usage);
> +	printf("%s", usage);

I dislike this change.  Which warning does the old code produce for
you?

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: wd at denx.de
Children begin by loving their parents. After a time they judge them.
Rarely, if ever, do they forgive them.                  - Oscar Wilde

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Sun, Apr 24, 2011 at 6:14 PM, Wolfgang Denk wrote:
> Joakim Tjernlund wrote:
>> --- a/examples/standalone/timer.c
>> +++ b/examples/standalone/timer.c
>> @@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
>> ? ? ? /* clear all events */
>> ? ? ? *hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
>>
>> - ? ? printf (usage);
>> + ? ? printf("%s", usage);
>
> I dislike this change. ?Which warning does the old code produce for
> you?

i imagine he is using one of those "security conscious" compilers that
warn when you try to printf with a dynamic argument as the format.  we
probably want to disable this stuff for u-boot since it doesnt make
much sense by adding -Wno-format-nonliteral and -Wno-format-security
when the compiler supports it.

as for this one particular change, it probably makes sense to change
it to puts(usage) anyways since the usage string contains no format
modifiers.  it'll be faster this way.  and the code should be written:
static const char usage[] = "...";

the current usage has useless overhead.
-mike

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/24 22:48:09:
>
> Dear Joakim Tjernlund,
>
> In message <1303633774-22961-4-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > Seems like recent linker/fpic changes broke a few 8xx boards.
> > This makes them build again.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >  board/LEOX/elpt860/u-boot.lds |    2 +-
> >  board/etx094/u-boot.lds       |    2 +-
> >  board/svm_sc8xx/u-boot.lds    |    2 +-
> >  board/tqc/tqm8xx/u-boot.lds   |    2 ++
> >  4 files changed, 5 insertions(+), 3 deletions(-)
>
> Sorry, but isn't this just papering over the original problem?
> In the reverted commit you claimed:
>
>     -msingle-pic-base is a new gcc option for ppc and
>     it reduces the size of my u-boot with 6-8 KB.
>     While at it, add -fno-jump-tables too to save a
>     few more bytes.
>
> This promises to reduce the overall code size, but instead the
> observation is that it is growing, which is the reson for the build
> problem on some boards.

Well, your gcc is different from mine and I know nothing about it as
you won't say anything about it.
This is what I get:

With -msingle-pic-base:

## 4.4.5 has been patched to support msingle-pic-base
CROSS_COMPILE=powerpc-4.4.5_softfloat-linux-gnu- ./MAKEALL  TQM855L TQM860L
Configuring for TQM855L board...
   text	   data	    bss	    dec	    hex	filename
 267022	   5284	  27200	 299506	  491f2	./u-boot
Configuring for TQM860L board...
   text	   data	    bss	    dec	    hex	filename
 267043	   5284	  27200	 299527	  49207	./u-boot

##powerpc-softfloat-linux-gnu- is 3.4.6
CROSS_COMPILE=powerpc-softfloat-linux-gnu- ./MAKEALL  TQM855L TQM860L
Configuring for TQM855L board...
u-boot.lds:90 cannot move location counter backwards (from 00000000400081d4 to 0000000040008000)
make: *** [u-boot] Error 1
powerpc-softfloat-linux-gnu-size: './u-boot': No such file
Configuring for TQM860L board...
u-boot.lds:90 cannot move location counter backwards (from 00000000400081e0 to 0000000040008000)
make: *** [u-boot] Error 1
powerpc-softfloat-linux-gnu-size: './u-boot': No such file

Reverted -msingle-pic-base:

CROSS_COMPILE=powerpc-4.4.5_softfloat-linux-gnu- ./MAKEALL  TQM855L TQM860L
Configuring for TQM855L board...
u-boot.lds:75 cannot move location counter backwards (from 40008180 to 40008000)
make: *** [u-boot] Error 1
powerpc-4.4.5_softfloat-linux-gnu-size: './u-boot': No such file
Configuring for TQM860L board...
u-boot.lds:75 cannot move location counter backwards (from 40008190 to 40008000)
make: *** [u-boot] Error 1
powerpc-4.4.5_softfloat-linux-gnu-size: './u-boot': No such file

CROSS_COMPILE=powerpc-softfloat-linux-gnu- ./MAKEALL  TQM855L TQM860L
Configuring for TQM855L board...
u-boot.lds:90 cannot move location counter backwards (from 00000000400081d4 to 0000000040008000)
make: *** [u-boot] Error 1
powerpc-softfloat-linux-gnu-size: './u-boot': No such file
Configuring for TQM860L board...
u-boot.lds:90 cannot move location counter backwards (from 00000000400081e0 to 0000000040008000)
make: *** [u-boot] Error 1
powerpc-softfloat-linux-gnu-size: './u-boot': No such file

So I strongly suspect that as soon as you move your gcc version upwards, it will break
these boards too.

> For example TQM860L (top of tree, with 8c4734e reverted (i. e. your
> old 39768f7 reapplied) and this patch here applied, I see:
>
>    text    data     bss     dec     hex filename
>  258663   14676   27228  300567   49617 ./u-boot
>
> and
>
>                 0x00008000                . = DEFINED (env_offset)?env_offset:.
>  *fill*         0x40007d28      0x2d8 00
>  common/env_embedded.o(.ppcenv*)
>  .ppcenv        0x40008000     0x8000 common/env_embedded.o
>
> Without these two changes we have:
>
>    text    data     bss     dec     hex filename
>  257955   14744   27228  299927   49397 ./u-boot
>
> and
>
>  670                 0x00008000                . = DEFINED (env_offset)?env_offset:.
>  671  *fill*         0x40007fb4       0x4c 00
>  672  common/env_embedded.o(.ppcenv*)
>  673  .ppcenv        0x40008000     0x8000 common/env_embedded.o
>
>
> The gap before the embedded environment which is just 76 bytes
> without your patches now grows to 728 bytes (i. e. by a factor of
> nearly 10), and the total image sie grows, too.

Of course, I had for these boards move something from the small space
before the environment as some gcc's produce larger code so the targeted
space was too small, something had to give.

>
>
> I will not apply either of these commits - neither the reverted one,
> nor this additional one, as both of them grow the memory footprint
> instead of redusing it as you promised in the commit message.

You should read the commit msg again. It requires a gcc which
supports -msingle-pic-base.
I still don't know why it broke with your gcc but I suspect
it is due to -fno-jump-tables(which is in the msingle-pic-base patch).
Seems like that option in your case makes the code bigger.

 Jocke

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/25 00:14:58:
>
> Dear Joakim Tjernlund,
>
> In message <1303633774-22961-3-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > Noticed while building all of mpc8xx
>
> Please include in the commit message what the compiler warnings were,
> and which compiler version was used to produce these warnings.
>
>
> > diff --git a/examples/standalone/timer.c b/examples/standalone/timer.c
> > index 834cc9a..a4d4581 100644
> > --- a/examples/standalone/timer.c
> > +++ b/examples/standalone/timer.c
> > @@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
> >     /* clear all events */
> >     *hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
> >
> > -   printf (usage);
> > +   printf("%s", usage);
>
> I dislike this change.  Which warning does the old code produce for
> you?

Updated patch below with the warnings in the commit msg:

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Joakim Tjernlund]

vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
>
> On Sun, Apr 24, 2011 at 6:14 PM, Wolfgang Denk wrote:
> > Joakim Tjernlund wrote:
> >> --- a/examples/standalone/timer.c
> >> +++ b/examples/standalone/timer.c
> >> @@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
> >> ? ? ? /* clear all events */
> >> ? ? ? *hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
> >>
> >> - ? ? printf (usage);
> >> + ? ? printf("%s", usage);
> >
> > I dislike this change. ?Which warning does the old code produce for
> > you?
>
> i imagine he is using one of those "security conscious" compilers that
> warn when you try to printf with a dynamic argument as the format.  we

Yes, if gcc 4.4.5 counst as "security conscious" :)

> probably want to disable this stuff for u-boot since it doesnt make
> much sense by adding -Wno-format-nonliteral and -Wno-format-security
> when the compiler supports it.
>
> as for this one particular change, it probably makes sense to change
> it to puts(usage) anyways since the usage string contains no format
> modifiers.  it'll be faster this way.  and the code should be written:
> static const char usage[] = "...";
>
> the current usage has useless overhead.

Yes, but puts() adds an newline so you can't just replace the above printf
with puts()

 Jocke

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
> vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
>> On Sun, Apr 24, 2011 at 6:14 PM, Wolfgang Denk wrote:
>> > Joakim Tjernlund wrote:
>> >> --- a/examples/standalone/timer.c
>> >> +++ b/examples/standalone/timer.c
>> >> @@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
>> >> ? ? ? /* clear all events */
>> >> ? ? ? *hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
>> >>
>> >> - ? ? printf (usage);
>> >> + ? ? printf("%s", usage);
>> >
>> > I dislike this change. ?Which warning does the old code produce for
>> > you?
>>
>> i imagine he is using one of those "security conscious" compilers that
>> warn when you try to printf with a dynamic argument as the format. ?we
>
> Yes, if gcc 4.4.5 counst as "security conscious" :)

your distro probably enables some default warning flags from the vanilla one

>> probably want to disable this stuff for u-boot since it doesnt make
>> much sense by adding -Wno-format-nonliteral and -Wno-format-security
>> when the compiler supports it.
>>
>> as for this one particular change, it probably makes sense to change
>> it to puts(usage) anyways since the usage string contains no format
>> modifiers. ?it'll be faster this way. ?and the code should be written:
>> static const char usage[] = "...";
>>
>> the current usage has useless overhead.
>
> Yes, but puts() adds an newline so you can't just replace the above printf
> with puts()

no, it doesnt.  u-boot's put() doesnt act the same as the standard C library.

however, that doesnt change my original point ... we shouldnt be
"fixing" things like this that have no relevance in the u-boot world.
disable the warning flags in the build system.
-mike

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Mon, Apr 25, 2011 at 12:13 AM, Mike Frysinger wrote:
> however, that doesnt change my original point ... we shouldnt be
> "fixing" things like this that have no relevance in the u-boot world.
> disable the warning flags in the build system.

see if this fixes your printf warnings:
--- a/config.mk
+++ b/config.mk
@@ -191,6 +191,8 @@ CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes
 endif

 CFLAGS += $(call cc-option,-fno-stack-protector)
+CFLAGS += $(call cc-option,-Wno-format-nonliteral)
+CFLAGS += $(call cc-option,-Wno-format-security)

 # $(CPPFLAGS) sets -g, which causes gcc to pass a suitable -g<format>
 # option to the assembler.
-mike

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Joakim Tjernlund]

vapierfilter at gmail.com wrote on 2011/04/25 06:13:20:
>
> On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
> > vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
> >> On Sun, Apr 24, 2011 at 6:14 PM, Wolfgang Denk wrote:
> >> > Joakim Tjernlund wrote:
> >> >> --- a/examples/standalone/timer.c
> >> >> +++ b/examples/standalone/timer.c
> >> >> @@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
> >> >> ? ? ? /* clear all events */
> >> >> ? ? ? *hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
> >> >>
> >> >> - ? ? printf (usage);
> >> >> + ? ? printf("%s", usage);
> >> >
> >> > I dislike this change. ?Which warning does the old code produce for
> >> > you?
> >>
> >> i imagine he is using one of those "security conscious" compilers that
> >> warn when you try to printf with a dynamic argument as the format. ?we
> >
> > Yes, if gcc 4.4.5 counst as "security conscious" :)
>
> your distro probably enables some default warning flags from the vanilla one
>
> >> probably want to disable this stuff for u-boot since it doesnt make
> >> much sense by adding -Wno-format-nonliteral and -Wno-format-security
> >> when the compiler supports it.
> >>
> >> as for this one particular change, it probably makes sense to change
> >> it to puts(usage) anyways since the usage string contains no format
> >> modifiers. ?it'll be faster this way. ?and the code should be written:
> >> static const char usage[] = "...";
> >>
> >> the current usage has useless overhead.
> >
> > Yes, but puts() adds an newline so you can't just replace the above printf
> > with puts()
>
> no, it doesnt.  u-boot's put() doesnt act the same as the standard C library.

Ah, didn't know that. I am not sure I like that especially
when there is a fputs in u-boot too.

   Jocke

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Joakim Tjernlund]

vapierfilter at gmail.com wrote on 2011/04/25 07:40:00:
>
> On Mon, Apr 25, 2011 at 12:13 AM, Mike Frysinger wrote:
> > however, that doesnt change my original point ... we shouldnt be
> > "fixing" things like this that have no relevance in the u-boot world.
> > disable the warning flags in the build system.
>
> see if this fixes your printf warnings:
> --- a/config.mk
> +++ b/config.mk
> @@ -191,6 +191,8 @@ CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes
>  endif
>
>  CFLAGS += $(call cc-option,-fno-stack-protector)
> +CFLAGS += $(call cc-option,-Wno-format-nonliteral)
> +CFLAGS += $(call cc-option,-Wno-format-security)

Yes, that removed the warning.

 Jocke

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF102CC99D.B86B21B6-ONC125787C.007CB439-C125787C.0080F875@transmode.se> you wrote:
>
> Well, your gcc is different from mine and I know nothing about it as
> you won't say anything about it.

If in doubt, use ELDK.  All examples were compiled with ELDK 4.2 (GCC
4.2.2) for ppc_8xx.

> ## 4.4.5 has been patched to support msingle-pic-base
> CROSS_COMPILE=powerpc-4.4.5_softfloat-linux-gnu- ./MAKEALL  TQM855L TQM860L
> Configuring for TQM855L board...
>    text	   data	    bss	    dec	    hex	filename
>  267022	   5284	  27200	 299506	  491f2	./u-boot
> Configuring for TQM860L board...
>    text	   data	    bss	    dec	    hex	filename
>  267043	   5284	  27200	 299527	  49207	./u-boot

I don't have gcc 4.4.x here for testing.

> So I strongly suspect that as soon as you move your gcc version upwards, it will break
> these boards too.

Yes, but for other reasons.

Recent versions of GCC (gcc 4.5.x) cannot be used to build this code
at all until they get fixed, as -Os is not working, and the code size
explodes completely. See GCC bugs 43810, 48278, and 45053.

> > I will not apply either of these commits - neither the reverted one,
> > nor this additional one, as both of them grow the memory footprint
> > instead of redusing it as you promised in the commit message.
> 
> You should read the commit msg again. It requires a gcc which
> supports -msingle-pic-base.

As the majority of gcc versions outthere does not support this
feature, we are trading a (slight?) advantage for a very limited group
against disadvantages for a very large group (probably everyone else
but you).

> I still don't know why it broke with your gcc but I suspect
> it is due to -fno-jump-tables(which is in the msingle-pic-base patch).
> Seems like that option in your case makes the code bigger.

Indeed. As I cannot see any advantages, I reject this stuff, at least
for now.

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: wd at denx.de
Due to lack of disk space, this fortune database has been discontinued.

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/25 10:35:25:
>
> Dear Joakim Tjernlund,
>
> In message <OF102CC99D.B86B21B6-ONC125787C.007CB439-C125787C.0080F875@transmode.se> you wrote:
> >
> > Well, your gcc is different from mine and I know nothing about it as
> > you won't say anything about it.
>
> If in doubt, use ELDK.  All examples were compiled with ELDK 4.2 (GCC
> 4.2.2) for ppc_8xx.

Have you tried gcc 4.3.x yet?

>
> > ## 4.4.5 has been patched to support msingle-pic-base
> > CROSS_COMPILE=powerpc-4.4.5_softfloat-linux-gnu- ./MAKEALL  TQM855L TQM860L
> > Configuring for TQM855L board...
> >    text      data       bss       dec       hex   filename
> >  267022      5284     27200    299506     491f2   ./u-boot
> > Configuring for TQM860L board...
> >    text      data       bss       dec       hex   filename
> >  267043      5284     27200    299527     49207   ./u-boot
>
> I don't have gcc 4.4.x here for testing.
>
> > So I strongly suspect that as soon as you move your gcc version upwards, it will break
> > these boards too.
>
> Yes, but for other reasons.

Rather for these reasons too :)

>
> Recent versions of GCC (gcc 4.5.x) cannot be used to build this code
> at all until they get fixed, as -Os is not working, and the code size
> explodes completely. See GCC bugs 43810, 48278, and 45053.

hmm, can't really make out if these TR:s also applies for gcc 4.4.5?
There are some hints but I cannot tell for sure.

>
> > > I will not apply either of these commits - neither the reverted one,
> > > nor this additional one, as both of them grow the memory footprint
> > > instead of redusing it as you promised in the commit message.
> >
> > You should read the commit msg again. It requires a gcc which
> > supports -msingle-pic-base.
>
> As the majority of gcc versions outthere does not support this
> feature, we are trading a (slight?) advantage for a very limited group
> against disadvantages for a very large group (probably everyone else
> but you).

That will probably change very soon. Gentoo will have -msingle-pic-base
in its next gcc bump of 4.4.x and 4.5.x

I think the mistake was to include the use of -fno-jump-tables in that
patch too. The -msingle-pic-base part should be a NOP to all gcc's that
don't have it.

>
> > I still don't know why it broke with your gcc but I suspect
> > it is due to -fno-jump-tables(which is in the msingle-pic-base patch).
> > Seems like that option in your case makes the code bigger.
>
> Indeed. As I cannot see any advantages, I reject this stuff, at least
> for now.

Indeed? Does that mean you confirmed that -fno-jump-tables was the cause?
I can respin the patch without that part if so.

 Jocke

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF522299D7.A02484A0-ONC125787D.003187D6-C125787D.0032F0AF@transmode.se> you wrote:
>
> > If in doubt, use ELDK.  All examples were compiled with ELDK 4.2 (GCC
> > 4.2.2) for ppc_8xx.
> 
> Have you tried gcc 4.3.x yet?

No, and from the list of bugs I have seen for it (especially for ARM)
I will not invest time and efforts on it.

> I think the mistake was to include the use of -fno-jump-tables in that
> patch too. The -msingle-pic-base part should be a NOP to all gcc's that
> don't have it.

Maybe.

> > > I still don't know why it broke with your gcc but I suspect
> > > it is due to -fno-jump-tables(which is in the msingle-pic-base patch).
> > > Seems like that option in your case makes the code bigger.
> >
> > Indeed. As I cannot see any advantages, I reject this stuff, at least
> > for now.
> 
> Indeed? Does that mean you confirmed that -fno-jump-tables was the cause?
> I can respin the patch without that part if so.

The "indeed" refers to the "in my case your patches make the code
bigger".  I cannot confirm anything else as I haven't tested it.

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: wd at denx.de
I have the simplest tastes.  I am always satisfied with the best.
                                                       -- Oscar Wilde

[U-Boot] [PATCH] Fix a few gcc warnings.

[Joakim Tjernlund]

Noticed while building all of mpc8xx. Also
constify usage string in timer.c
Warnings fixed are:
timer.c: In function 'timer':
timer.c:189: warning: format not a string literal and no format arguments
timer.c:258: warning: format not a string literal and no format arguments
atm.c: In function 'atmUnload':
atm.c:99: warning: array subscript is above array bounds
atm.c: In function 'atmLoad':
atm.c:65: warning: array subscript is above array bounds
codec.c: In function 'codsp_write_pop_int':
codec.c:678: warning: array subscript is above array bounds
codec.c: In function 'codsp_write_cop_short':
codec.c:585: warning: array subscript is above array bounds
codec.c: In function 'codsp_write_sop_int':
codec.c:512: warning: array subscript is above array bounds

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---

 -V3 use puts() and constify usage string in timer.c

 board/netta/codec.c         |    6 +++---
 board/siemens/IAD210/atm.c  |    4 ++--
 examples/standalone/timer.c |    6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/board/netta/codec.c b/board/netta/codec.c
index 844aa18..c8d31d7 100644
--- a/board/netta/codec.c
+++ b/board/netta/codec.c
@@ -502,7 +502,7 @@ void codsp_write_sop_short(int duslic_id, int channel, unsigned char regno, unsi
 
 void codsp_write_sop_int(int duslic_id, int channel, unsigned char regno, unsigned int val)
 {
-	unsigned char cmd[5];
+	unsigned char cmd[6];
 
 	cmd[0] = CODSP_WR | CODSP_ADR(channel) | CODSP_CMD_SOP;
 	cmd[1] = regno;
@@ -577,7 +577,7 @@ void codsp_write_cop_char(int duslic_id, int channel, unsigned char addr, unsign
 
 void codsp_write_cop_short(int duslic_id, int channel, unsigned char addr, unsigned short val)
 {
-	unsigned char cmd[3];
+	unsigned char cmd[4];
 
 	cmd[0] = CODSP_WR | CODSP_OP | CODSP_ADR(channel) | CODSP_CMD_COP;
 	cmd[1] = addr;
@@ -668,7 +668,7 @@ void codsp_write_pop_short (int duslic_id, int channel, unsigned char regno,
 void codsp_write_pop_int (int duslic_id, int channel, unsigned char regno,
 			  unsigned int val)
 {
-	unsigned char cmd[5];
+	unsigned char cmd[6];
 
 	cmd[0] = CODSP_WR | CODSP_ADR (channel) | CODSP_CMD_POP;
 	cmd[1] = regno;
diff --git a/board/siemens/IAD210/atm.c b/board/siemens/IAD210/atm.c
index e599c10..40aad0a 100644
--- a/board/siemens/IAD210/atm.c
+++ b/board/siemens/IAD210/atm.c
@@ -62,7 +62,7 @@ int atmLoad()
   volatile iop8xx_t      *iop    = &immap->im_ioport;
 
   timers->cpmt_tgcr &=  0x0FFF; SYNC;             /* Disable Timer 4 */
-  immap->im_cpm.cp_scc[4].scc_gsmrl = 0x0; SYNC; /* Disable SCC4 */
+  immap->im_cpm.cp_scc[3].scc_gsmrl = 0x0; SYNC; /* Disable SCC4 */
   iop->iop_pdpar &= 0x3FFF; SYNC;                 /* Disable SAR and UTOPIA */
 
   if ( atmMemInit() != OK ) return ERROR;
@@ -96,7 +96,7 @@ void atmUnload()
   volatile iop8xx_t      *iop    = &immap->im_ioport;
 
   timers->cpmt_tgcr &=  0x0FFF; SYNC;             /* Disable Timer 4 */
-  immap->im_cpm.cp_scc[4].scc_gsmrl = 0x0; SYNC;  /* Disable SCC4 */
+  immap->im_cpm.cp_scc[3].scc_gsmrl = 0x0; SYNC;  /* Disable SCC4 */
   iop->iop_pdpar &= 0x3FFF; SYNC;                 /* Disable SAR and UTOPIA */
   g_atm.loaded = FALSE;
 }
diff --git a/examples/standalone/timer.c b/examples/standalone/timer.c
index 834cc9a..8fd38b2 100644
--- a/examples/standalone/timer.c
+++ b/examples/standalone/timer.c
@@ -113,7 +113,7 @@ typedef struct tid_8xx_cpmtimer_s {
 
 void setPeriod (tid_8xx_cpmtimer_t *hwp, ulong interval);
 
-static char *usage = "\n[q, b, e, ?] ";
+static const char usage[] = "\n[q, b, e, ?] ";
 
 int timer (int argc, char * const argv[])
 {
@@ -186,7 +186,7 @@ int timer (int argc, char * const argv[])
 	/* clear all events */
 	*hwp->terp = (CPMT_EVENT_CAP | CPMT_EVENT_REF);
 
-	printf (usage);
+	puts(usage);
 	running = 0;
 	while ((c = getc()) != 'q') {
 	    if (c == 'b') {
@@ -255,7 +255,7 @@ int timer (int argc, char * const argv[])
 	    } else {
 		printf ("\nEnter: q - quit, b - start timer, e - stop timer, ? - get status\n");
 	    }
-	    printf (usage);
+	    puts(usage);
 	}
 	if (running) {
 		printf ("Stopping timer\n");
-- 
1.7.3.4

[U-Boot] [PATCH 3/3] powerpc, 8xx: adapt a few linker scripts so it will build.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/04/25 12:17:11:
>
> Dear Joakim Tjernlund,
>
> In message <OF522299D7.A02484A0-ONC125787D.003187D6-C125787D.0032F0AF@transmode.se> you wrote:
> >
> > > If in doubt, use ELDK.  All examples were compiled with ELDK 4.2 (GCC
> > > 4.2.2) for ppc_8xx.
> >
> > Have you tried gcc 4.3.x yet?
>
> No, and from the list of bugs I have seen for it (especially for ARM)
> I will not invest time and efforts on it.
>
> > I think the mistake was to include the use of -fno-jump-tables in that
> > patch too. The -msingle-pic-base part should be a NOP to all gcc's that
> > don't have it.
>
> Maybe.
>
> > > > I still don't know why it broke with your gcc but I suspect
> > > > it is due to -fno-jump-tables(which is in the msingle-pic-base patch).
> > > > Seems like that option in your case makes the code bigger.
> > >
> > > Indeed. As I cannot see any advantages, I reject this stuff, at least
> > > for now.
> >
> > Indeed? Does that mean you confirmed that -fno-jump-tables was the cause?
> > I can respin the patch without that part if so.
>
> The "indeed" refers to the "in my case your patches make the code
> bigger".  I cannot confirm anything else as I haven't tested it.

Dare I ask you try this revised patch with -fno-jump-tables removed?

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Mon, Apr 25, 2011 at 04:30, Joakim Tjernlund wrote:
> vapierfilter at gmail.com wrote on 2011/04/25 06:13:20:
>> On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
>> > Yes, but puts() adds an newline so you can't just replace the above printf
>> > with puts()
>>
>> no, it doesnt. ?u-boot's put() doesnt act the same as the standard C library.
>
> Ah, didn't know that. I am not sure I like that especially
> when there is a fputs in u-boot too.

cant say i'm a big fan of it either, but i think it's way too buried
in the fabric of u-boot to be worth trying to change now.  and it is
slightly more flexible than the C library puts() in that you can
puts() multiple partial strings without worrying about the newline
interjection ...
-mike

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Scott Wood]

On Mon, 25 Apr 2011 00:13:20 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
> > vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
> >> probably want to disable this stuff for u-boot since it doesnt make
> >> much sense by adding -Wno-format-nonliteral and -Wno-format-security
> >> when the compiler supports it.
> >>
> >> as for this one particular change, it probably makes sense to change
> >> it to puts(usage) anyways since the usage string contains no format
> >> modifiers. ?it'll be faster this way. ?and the code should be written:
> >> static const char usage[] = "...";
> >>
> >> the current usage has useless overhead.
> >
> > Yes, but puts() adds an newline so you can't just replace the above printf
> > with puts()
> 
> no, it doesnt.  u-boot's put() doesnt act the same as the standard C library.
> 
> however, that doesnt change my original point ... we shouldnt be
> "fixing" things like this that have no relevance in the u-boot world.
> disable the warning flags in the build system.

Why encourage bad habits?  Are there any instances of this in U-Boot where
conversion to puts() wouldn't be an improvement, especially given the lack
of an automatic newline in U-Boot's version?

If we have an instance where we really want formatting on a dynamic string,
that's another matter.

-Scott

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Mon, Apr 25, 2011 at 13:45, Scott Wood wrote:
> On Mon, 25 Apr 2011 00:13:20 -0400 Mike Frysinger wrote:
>> On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
>> > vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
>> >> probably want to disable this stuff for u-boot since it doesnt make
>> >> much sense by adding -Wno-format-nonliteral and -Wno-format-security
>> >> when the compiler supports it.
>> >>
>> >> as for this one particular change, it probably makes sense to change
>> >> it to puts(usage) anyways since the usage string contains no format
>> >> modifiers. ?it'll be faster this way. ?and the code should be written:
>> >> static const char usage[] = "...";
>> >>
>> >> the current usage has useless overhead.
>> >
>> > Yes, but puts() adds an newline so you can't just replace the above printf
>> > with puts()
>>
>> no, it doesnt. ?u-boot's put() doesnt act the same as the standard C library.
>>
>> however, that doesnt change my original point ... we shouldnt be
>> "fixing" things like this that have no relevance in the u-boot world.
>> disable the warning flags in the build system.
>
> Why encourage bad habits? ?Are there any instances of this in U-Boot where
> conversion to puts() wouldn't be an improvement, especially given the lack
> of an automatic newline in U-Boot's version?

that wasnt what i was saying.  my point is simply that changing
printf(foo); to printf("%s", foo); simply to satisfy a gcc warning is
wrong and unnecessarily bloats the compiled code.  if you want to
change it from printf(foo) to puts(foo), that's fine by me (and is
actually what i suggested).
-mike

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Joakim Tjernlund]

vapierfilter at gmail.com wrote on 2011/04/25 19:53:50:
>
> On Mon, Apr 25, 2011 at 13:45, Scott Wood wrote:
> > On Mon, 25 Apr 2011 00:13:20 -0400 Mike Frysinger wrote:
> >> On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
> >> > vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
> >> >> probably want to disable this stuff for u-boot since it doesnt make
> >> >> much sense by adding -Wno-format-nonliteral and -Wno-format-security
> >> >> when the compiler supports it.
> >> >>
> >> >> as for this one particular change, it probably makes sense to change
> >> >> it to puts(usage) anyways since the usage string contains no format
> >> >> modifiers. ?it'll be faster this way. ?and the code should be written:
> >> >> static const char usage[] = "...";
> >> >>
> >> >> the current usage has useless overhead.
> >> >
> >> > Yes, but puts() adds an newline so you can't just replace the above printf
> >> > with puts()
> >>
> >> no, it doesnt. ?u-boot's put() doesnt act the same as the standard C library.
> >>
> >> however, that doesnt change my original point ... we shouldnt be
> >> "fixing" things like this that have no relevance in the u-boot world.
> >> disable the warning flags in the build system.
> >
> > Why encourage bad habits? ?Are there any instances of this in U-Boot where
> > conversion to puts() wouldn't be an improvement, especially given the lack
> > of an automatic newline in U-Boot's version?
>
> that wasnt what i was saying.  my point is simply that changing
> printf(foo); to printf("%s", foo); simply to satisfy a gcc warning is
> wrong and unnecessarily bloats the compiled code.  if you want to
> change it from printf(foo) to puts(foo), that's fine by me (and is
> actually what i suggested).
> -mike

Which is what I did in the second version I sent in a few hours ago :)

 Jocke

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Mon, Apr 25, 2011 at 13:58, Joakim Tjernlund wrote:
> vapierfilter at gmail.com wrote on 2011/04/25 19:53:50:
>> On Mon, Apr 25, 2011 at 13:45, Scott Wood wrote:
>> > On Mon, 25 Apr 2011 00:13:20 -0400 Mike Frysinger wrote:
>> >> On Sun, Apr 24, 2011 at 7:42 PM, Joakim Tjernlund wrote:
>> >> > vapierfilter at gmail.com wrote on 2011/04/25 00:38:31:
>> >> >> probably want to disable this stuff for u-boot since it doesnt make
>> >> >> much sense by adding -Wno-format-nonliteral and -Wno-format-security
>> >> >> when the compiler supports it.
>> >> >>
>> >> >> as for this one particular change, it probably makes sense to change
>> >> >> it to puts(usage) anyways since the usage string contains no format
>> >> >> modifiers. ?it'll be faster this way. ?and the code should be written:
>> >> >> static const char usage[] = "...";
>> >> >>
>> >> >> the current usage has useless overhead.
>> >> >
>> >> > Yes, but puts() adds an newline so you can't just replace the above printf
>> >> > with puts()
>> >>
>> >> no, it doesnt. ?u-boot's put() doesnt act the same as the standard C library.
>> >>
>> >> however, that doesnt change my original point ... we shouldnt be
>> >> "fixing" things like this that have no relevance in the u-boot world.
>> >> disable the warning flags in the build system.
>> >
>> > Why encourage bad habits? ?Are there any instances of this in U-Boot where
>> > conversion to puts() wouldn't be an improvement, especially given the lack
>> > of an automatic newline in U-Boot's version?
>>
>> that wasnt what i was saying. ?my point is simply that changing
>> printf(foo); to printf("%s", foo); simply to satisfy a gcc warning is
>> wrong and unnecessarily bloats the compiled code. ?if you want to
>> change it from printf(foo) to puts(foo), that's fine by me (and is
>> actually what i suggested).
>
> Which is what I did in the second version I sent in a few hours ago :)

yes, but this response was just for Scott
-mike

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Scott Wood]

On Mon, 25 Apr 2011 13:53:50 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> On Mon, Apr 25, 2011 at 13:45, Scott Wood wrote:
> > Why encourage bad habits? ?Are there any instances of this in U-Boot where
> > conversion to puts() wouldn't be an improvement, especially given the lack
> > of an automatic newline in U-Boot's version?
> 
> that wasnt what i was saying.  my point is simply that changing
> printf(foo); to printf("%s", foo); simply to satisfy a gcc warning is
> wrong and unnecessarily bloats the compiled code.

My point was that the warning should stay, until such a time as it's
complaining about something that we actually want to do -- I've yet to see
an example cited so far that didn't have an easy non-"bloating" fix.

Even in a context such as U-Boot, IMHO format-string functions shouldn't be
used in such a way -- it may not be a security issue, but it's a potential
readability/maintainability issue when it may not be obvious from the other
context where the string is defined, that any embedded percent characters
must be doubled.  It's a different situation from where a string literal is
defined right in the context of the format-string function.

>  if you want to
> change it from printf(foo) to puts(foo), that's fine by me (and is
> actually what i suggested).

And after writing this, you sent a patch changing the warning options...

-Scott

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Mike Frysinger]

On Mon, Apr 25, 2011 at 15:45, Scott Wood wrote:
> And after writing this, you sent a patch changing the warning options...

i sent a proper patch to do a suggestion i already made here.  and it
isnt really changing anything ... most toolchains already have this
behavior by default.  my patch only changes it for people who are
using non-standard toolchains, and they are clearly the minority as
the majority of people writing & submitting patches arent seeing the
warnings.
-mike

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Wolfgang Denk]

Dear Scott Wood,

In message <20110425144518.5a37bd4c@schlenkerla.am.freescale.net> you wrote:
>
> Mike Frysinger <vapier@gentoo.org> wrote:
...
> > that wasnt what i was saying.  my point is simply that changing
> > printf(foo); to printf("%s", foo); simply to satisfy a gcc warning is
> > wrong and unnecessarily bloats the compiled code.
> 
> My point was that the warning should stay, until such a time as it's
> complaining about something that we actually want to do -- I've yet to see
> an example cited so far that didn't have an easy non-"bloating" fix.

I disagree. "printf(foo);" may be suboptimal but there are cases
where I do not want to see a warning about this. Consider for example
common/main.c:

 115 #  ifdef CONFIG_AUTOBOOT_PROMPT
 116         printf(CONFIG_AUTOBOOT_PROMPT);
 117 #  endif

Here we provide a way for a user-defined autoboot prompt message. Some
users may just want to provide a plain string - what's wrong with
that?  [Yes, there are other ways to implement this, but why make it
more complicated than necessary?]

> Even in a context such as U-Boot, IMHO format-string functions shouldn't be
> used in such a way -- it may not be a security issue, but it's a potential
> readability/maintainability issue when it may not be obvious from the other
> context where the string is defined, that any embedded percent characters
> must be doubled.  It's a different situation from where a string literal is
> defined right in the context of the format-string function.

Again, I disagree.


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: wd at denx.de
I don't mind criticism. You know me. I've  never  been  one  to  take
offence  at  criticism. No one could say I'm the sort to take offence
at criticism -- Not twice, anyway. Not without blowing bubbles.
                                  - Terry Pratchett, _Witches Abroad_

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Scott Wood]

On Mon, 25 Apr 2011 23:23:41 +0200
Wolfgang Denk <wd@denx.de> wrote:

> Dear Scott Wood,
> 
> In message <20110425144518.5a37bd4c@schlenkerla.am.freescale.net> you wrote:
> >
> > Mike Frysinger <vapier@gentoo.org> wrote:
> ...
> > > that wasnt what i was saying.  my point is simply that changing
> > > printf(foo); to printf("%s", foo); simply to satisfy a gcc warning is
> > > wrong and unnecessarily bloats the compiled code.
> > 
> > My point was that the warning should stay, until such a time as it's
> > complaining about something that we actually want to do -- I've yet to see
> > an example cited so far that didn't have an easy non-"bloating" fix.
> 
> I disagree. "printf(foo);" may be suboptimal but there are cases
> where I do not want to see a warning about this. Consider for example
> common/main.c:
> 
>  115 #  ifdef CONFIG_AUTOBOOT_PROMPT
>  116         printf(CONFIG_AUTOBOOT_PROMPT);
>  117 #  endif
> 
> Here we provide a way for a user-defined autoboot prompt message. Some
> users may just want to provide a plain string - what's wrong with
> that?  [Yes, there are other ways to implement this, but why make it
> more complicated than necessary?]

It won't warn there, because it all happens in the preprocessor.

-Scott

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Wolfgang Denk]

Dear Scott Wood,

In message <20110425162854.05500a06@schlenkerla.am.freescale.net> you wrote:
>
> > I disagree. "printf(foo);" may be suboptimal but there are cases
> > where I do not want to see a warning about this. Consider for example
> > common/main.c:
> > 
> >  115 #  ifdef CONFIG_AUTOBOOT_PROMPT
> >  116         printf(CONFIG_AUTOBOOT_PROMPT);
> >  117 #  endif
> > 
> > Here we provide a way for a user-defined autoboot prompt message. Some
> > users may just want to provide a plain string - what's wrong with
> > that?  [Yes, there are other ways to implement this, but why make it
> > more complicated than necessary?]
> 
> It won't warn there, because it all happens in the preprocessor.

Why would it not warn if the user just does

	#define CONFIG_AUTOBOOT_PROMPT "my prompt:"

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: wd at denx.de
How much net work could a network work, if a network could net work?

[U-Boot] [PATCH 2/3] Fix a few gcc warnings.

[Scott Wood]

On Tue, 26 Apr 2011 00:29:06 +0200
Wolfgang Denk <wd@denx.de> wrote:

> Dear Scott Wood,
> 
> In message <20110425162854.05500a06@schlenkerla.am.freescale.net> you wrote:
> >
> > > I disagree. "printf(foo);" may be suboptimal but there are cases
> > > where I do not want to see a warning about this. Consider for example
> > > common/main.c:
> > > 
> > >  115 #  ifdef CONFIG_AUTOBOOT_PROMPT
> > >  116         printf(CONFIG_AUTOBOOT_PROMPT);
> > >  117 #  endif
> > > 
> > > Here we provide a way for a user-defined autoboot prompt message. Some
> > > users may just want to provide a plain string - what's wrong with
> > > that?  [Yes, there are other ways to implement this, but why make it
> > > more complicated than necessary?]
> > 
> > It won't warn there, because it all happens in the preprocessor.
> 
> Why would it not warn if the user just does
> 
> 	#define CONFIG_AUTOBOOT_PROMPT "my prompt:"

Because the warning only applies to non-literals (preprocessor
substitution doesn't count, as it's a literal when the preprocessor's done
with it).  Otherwise it would be warning on every instance of
printf("foo\n").

-Scott

[U-Boot] [PATCH] Fix a few gcc warnings.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1303731583-11353-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Noticed while building all of mpc8xx. Also
> constify usage string in timer.c
> Warnings fixed are:
> timer.c: In function 'timer':
> timer.c:189: warning: format not a string literal and no format arguments
> timer.c:258: warning: format not a string literal and no format arguments
> atm.c: In function 'atmUnload':
> atm.c:99: warning: array subscript is above array bounds
> atm.c: In function 'atmLoad':
> atm.c:65: warning: array subscript is above array bounds
> codec.c: In function 'codsp_write_pop_int':
> codec.c:678: warning: array subscript is above array bounds
> codec.c: In function 'codsp_write_cop_short':
> codec.c:585: warning: array subscript is above array bounds
> codec.c: In function 'codsp_write_sop_int':
> codec.c:512: warning: array subscript is above array bounds
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> 
>  -V3 use puts() and constify usage string in timer.c
> 
>  board/netta/codec.c         |    6 +++---
>  board/siemens/IAD210/atm.c  |    4 ++--
>  examples/standalone/timer.c |    6 +++---
>  3 files changed, 8 insertions(+), 8 deletions(-)

Applied, thanks.

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: wd at denx.de
				 EARTH
			     smog  |   bricks
			 AIR  --  mud  --  FIRE
			soda water |   tequila
				 WATER