[PATCH] ARM: Fix uncompress code compile for different defines of static(void)

[PATCH] ARM: Fix uncompress code compile for different defines of static(void)

[Tony Lindgren]

Because of the include of the decompress_inflate.c file from
boot/compress/misc.c, there are different flush() defines:

In file included from arch/arm/boot/compressed/misc.c:249:
arch/arm/boot/compressed/../../../../lib/decompress_inflate.c:138:29: error: macro "flush" passed 2 arguments, but takes just 0

Fix this by removing the define of flush() in misc.c for
CONFIG_DEBUG_ICEDCC as it's already defined in mach/uncompress.h,
and that is being included unconditionally.

Also use a static inline function instead of define
for mach-mxc and mach-gemini to avoid similar bug
for those platforms.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -100,7 +100,6 @@ static void icedcc_putc(int ch)
 #endif
 
 #define putc(ch)	icedcc_putc(ch)
-#define flush()	do { } while (0)
 #endif
 
 static void putstr(const char *ptr)
--- a/arch/arm/mach-gemini/include/mach/uncompress.h
+++ b/arch/arm/mach-gemini/include/mach/uncompress.h
@@ -30,7 +30,9 @@ static inline void putc(char c)
 	UART[UART_TX] = c;
 }
 
-#define flush() do { } while (0)
+static inline void flush(void)
+{
+}
 
 /*
  * nothing to do
--- a/arch/arm/plat-mxc/include/mach/uncompress.h
+++ b/arch/arm/plat-mxc/include/mach/uncompress.h
@@ -60,7 +60,9 @@ static void putc(int ch)
 	UART(TXR) = ch;
 }
 
-#define flush() do { } while (0)
+static inline void flush(void)
+{
+}
 
 #define MX1_UART1_BASE_ADDR	0x00206000
 #define MX25_UART1_BASE_ADDR	0x43f90000

[PATCH] ARM: Fix uncompress code compile for different defines of static(void)

[Uwe Kleine-König]

Hello,

On Tue, Jan 12, 2010 at 06:19:40PM -0800, Tony Lindgren wrote:
> Because of the include of the decompress_inflate.c file from
> boot/compress/misc.c, there are different flush() defines:
> 
> In file included from arch/arm/boot/compressed/misc.c:249:
> arch/arm/boot/compressed/../../../../lib/decompress_inflate.c:138:29: error: macro "flush" passed 2 arguments, but takes just 0
> 
> Fix this by removing the define of flush() in misc.c for
> CONFIG_DEBUG_ICEDCC as it's already defined in mach/uncompress.h,
> and that is being included unconditionally.
> 
> Also use a static inline function instead of define
> for mach-mxc and mach-gemini to avoid similar bug
> for those platforms.
As arch/arm/boot/compressed/misc.c is compiled with -Dstatic= and this
is AFAIK the only user of uncompress.h I'd skip "static" and/or add a
comment telling that static is redundant here.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

[PATCH] ARM: Fix uncompress code compile for different defines of static(void)

[Uwe Kleine-König]

Hello,

On Wed, Jan 13, 2010 at 09:49:37AM +0100, Uwe Kleine-K?nig wrote:
> On Tue, Jan 12, 2010 at 06:19:40PM -0800, Tony Lindgren wrote:
> > Because of the include of the decompress_inflate.c file from
> > boot/compress/misc.c, there are different flush() defines:
> > 
> > In file included from arch/arm/boot/compressed/misc.c:249:
> > arch/arm/boot/compressed/../../../../lib/decompress_inflate.c:138:29: error: macro "flush" passed 2 arguments, but takes just 0
> > 
> > Fix this by removing the define of flush() in misc.c for
> > CONFIG_DEBUG_ICEDCC as it's already defined in mach/uncompress.h,
> > and that is being included unconditionally.
> > 
> > Also use a static inline function instead of define
> > for mach-mxc and mach-gemini to avoid similar bug
> > for those platforms.
> As arch/arm/boot/compressed/misc.c is compiled with -Dstatic= and this
> is AFAIK the only user of uncompress.h I'd skip "static" and/or add a
> comment telling that static is redundant here.
while reading my reply I noticed that I don't understand the subject.
"different defines of static(void)"?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

[PATCH] ARM: Fix uncompress code compile for different defines of static(void)

[Tony Lindgren]

* Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> [100113 06:24]:
> Hello,
> 
> On Wed, Jan 13, 2010 at 09:49:37AM +0100, Uwe Kleine-K?nig wrote:
> > On Tue, Jan 12, 2010 at 06:19:40PM -0800, Tony Lindgren wrote:
> > > Because of the include of the decompress_inflate.c file from
> > > boot/compress/misc.c, there are different flush() defines:
> > > 
> > > In file included from arch/arm/boot/compressed/misc.c:249:
> > > arch/arm/boot/compressed/../../../../lib/decompress_inflate.c:138:29: error: macro "flush" passed 2 arguments, but takes just 0
> > > 
> > > Fix this by removing the define of flush() in misc.c for
> > > CONFIG_DEBUG_ICEDCC as it's already defined in mach/uncompress.h,
> > > and that is being included unconditionally.
> > > 
> > > Also use a static inline function instead of define
> > > for mach-mxc and mach-gemini to avoid similar bug
> > > for those platforms.
> > As arch/arm/boot/compressed/misc.c is compiled with -Dstatic= and this
> > is AFAIK the only user of uncompress.h I'd skip "static" and/or add a
> > comment telling that static is redundant here.

Maybe we should do another patch for the merge window to remove
the static for flus from all uncompress.h files?

> while reading my reply I noticed that I don't understand the subject.
> "different defines of static(void)"?

Oops, typo. The subject should have been flush(void) instead of
static(void) :)

There's int(*flush)(void*, unsigned int) in lib/decompress_inflate.c,
and there's static inline void flush(void) in uncompress.h.

As decompress_inflate.c is included into misc.c, then the define
flush() do { } while (0) causes confusion.

Regards,

Tony

[PATCH] ARM: Fix uncompress code compile for different definesof static(void)

[H Hartley Sweeten]

On Wednesday, January 13, 2010 1:50 AM, Uwe Kleine-K?nig wrote:
>> Also use a static inline function instead of define
>> for mach-mxc and mach-gemini to avoid similar bug
>> for those platforms.
> As arch/arm/boot/compressed/misc.c is compiled with -Dstatic= and this
> is AFAIK the only user of uncompress.h I'd skip "static" and/or add a
> comment telling that static is redundant here.

So that's what causes the sparse warning!

Compiling misc.c produces a bunch of sparse warnings to the type:

arch/arm/boot/compressed/misc.c:19:14: warning: symbol '__machine_arch_type' was not declared. Should it be static?
arch/arm/mach-ep93xx/include/mach/uncompress.h:75:13: warning: symbol 'ethernet_reset' was not declared. Should it be static?
arch/arm/mach-ep93xx/include/mach/uncompress.h:109:13: warning: symbol 'enable_early_uart' was not declared. Should it be static?
arch/arm/mach-ep93xx/include/mach/uncompress.h:152:13: warning: symbol 'arch_decomp_setup' was not declared. Should it be static?
arch/arm/boot/compressed/misc.c:201:12: warning: symbol 'inbuf' was not declared. Should it be static?
arch/arm/boot/compressed/misc.c:202:12: warning: symbol 'window' was not declared. Should it be static?
arch/arm/boot/compressed/misc.c:204:17: warning: symbol 'insize' was not declared. Should it be static?
arch/arm/boot/compressed/misc.c:205:17: warning: symbol 'inptr' was not declared. Should it be static?
arch/arm/boot/compressed/misc.c:206:17: warning: symbol 'outcnt' was not declared. Should it be static?

Etc..

Any ideas on how to suppress the warnings?

Regards,
Hartley

[PATCH] ARM: Fix uncompress code compile for different definesof static(void)

[Uwe Kleine-König]

Hello Hartley,

On Wed, Jan 13, 2010 at 01:11:49PM -0500, H Hartley Sweeten wrote:
> On Wednesday, January 13, 2010 1:50 AM, Uwe Kleine-K?nig wrote:
> >> Also use a static inline function instead of define
> >> for mach-mxc and mach-gemini to avoid similar bug
> >> for those platforms.
> > As arch/arm/boot/compressed/misc.c is compiled with -Dstatic= and this
> > is AFAIK the only user of uncompress.h I'd skip "static" and/or add a
> > comment telling that static is redundant here.
> 
> So that's what causes the sparse warning!
> 
> Compiling misc.c produces a bunch of sparse warnings to the type:
> 
> arch/arm/boot/compressed/misc.c:19:14: warning: symbol '__machine_arch_type' was not declared. Should it be static?
> arch/arm/mach-ep93xx/include/mach/uncompress.h:75:13: warning: symbol 'ethernet_reset' was not declared. Should it be static?
> arch/arm/mach-ep93xx/include/mach/uncompress.h:109:13: warning: symbol 'enable_early_uart' was not declared. Should it be static?
> arch/arm/mach-ep93xx/include/mach/uncompress.h:152:13: warning: symbol 'arch_decomp_setup' was not declared. Should it be static?
> arch/arm/boot/compressed/misc.c:201:12: warning: symbol 'inbuf' was not declared. Should it be static?
> arch/arm/boot/compressed/misc.c:202:12: warning: symbol 'window' was not declared. Should it be static?
> arch/arm/boot/compressed/misc.c:204:17: warning: symbol 'insize' was not declared. Should it be static?
> arch/arm/boot/compressed/misc.c:205:17: warning: symbol 'inptr' was not declared. Should it be static?
> arch/arm/boot/compressed/misc.c:206:17: warning: symbol 'outcnt' was not declared. Should it be static?
> 
> Etc..
> 
> Any ideas on how to suppress the warnings?
I think the cleanest solution would be to remove -Dstatic=.  I don't
know the exact reason for it being there.  The best reference I found is

	http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg97485.html

Russell, maybe these toolchain problems are gone in the meantime?  Some
time ago I tried removing that and the decompressor was still
functional---which doesn't mean it's save in all situations with all
toolchains.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

[PATCH] ARM: Fix uncompress code compile for different definesof static(void)

[Russell King - ARM Linux]

On Thu, Jan 14, 2010 at 09:29:52AM +0100, Uwe Kleine-K?nig wrote:
> I think the cleanest solution would be to remove -Dstatic=.  I don't
> know the exact reason for it being there.

Thereby breaking the ability to build a relocatable decompressor.

> Russell, maybe these toolchain problems are gone in the meantime?

I don't think it was toolchain problems.  Having the text and data
segments independently relocatable is actually an abuse of the ELF
format - it's not something it directly supports.

[PATCH] ARM: Fix uncompress code compile for different definesof static(void)

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [100114 01:14]:
> On Thu, Jan 14, 2010 at 09:29:52AM +0100, Uwe Kleine-K?nig wrote:
> > I think the cleanest solution would be to remove -Dstatic=.  I don't
> > know the exact reason for it being there.
> 
> Thereby breaking the ability to build a relocatable decompressor.
> 
> > Russell, maybe these toolchain problems are gone in the meantime?
> 
> I don't think it was toolchain problems.  Having the text and data
> segments independently relocatable is actually an abuse of the ELF
> format - it's not something it directly supports.
> 
> From what I remember, having 'static' in there results in either
> function calls or static data being indirected through the GOT table,
> which means it has to be relocated along side the data segment - which
> makes the offset between the text and data segments fixed.
> 
> I would not believe that the "problems" have gone because the toolchain
> is working how it's supposed to.
> 
> The -Dstatic= stays.

I've added the compile fix with the subject fixed into Russell's
patch system as 5882/1.

Regards,

Tony

[PATCH] ARM: Fix uncompress code compile for different definesof static(void)

[Uwe Kleine-König]

Hello Russell,

On Thu, Jan 14, 2010 at 11:38:15AM -0800, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [100114 01:14]:
> > On Thu, Jan 14, 2010 at 09:29:52AM +0100, Uwe Kleine-K?nig wrote:
> > > I think the cleanest solution would be to remove -Dstatic=.  I don't
> > > know the exact reason for it being there.
> > 
> > Thereby breaking the ability to build a relocatable decompressor.
> > 
> > > Russell, maybe these toolchain problems are gone in the meantime?
> > 
> > I don't think it was toolchain problems.  Having the text and data
> > segments independently relocatable is actually an abuse of the ELF
> > format - it's not something it directly supports.
> > 
> > From what I remember, having 'static' in there results in either
> > function calls or static data being indirected through the GOT table,
> > which means it has to be relocated along side the data segment - which
> > makes the offset between the text and data segments fixed.
> > 
> > I would not believe that the "problems" have gone because the toolchain
> > is working how it's supposed to.
> > 
> > The -Dstatic= stays.
> 
> I've added the compile fix with the subject fixed into Russell's
> patch system as 5882/1.
can you please take this patch as currently all imx based machines fail
the compile test because of this.

You can interpret this as an Acked-by: or Tested-by: or both.

Thanks
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |