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

PATCH New Tony Lindgren 4 participants 9 messages Jan 13, 2010 – Jan 18, 2010

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Arm/cortina systems gemini Arm architecture

lore

Share & tools

lore

Tony Lindgren

author

2010-01-13 02:19 UTC #0 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different defines of static(void)

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 <[email protected]>

arch/arm/boot/compressed/misc.c +0 -1

@@ -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)

arch/arm/mach-gemini/include/mach/uncompress.h +3 -1

@@ -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

arch/arm/plat-mxc/include/mach/uncompress.h +3 -1

@@ -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

Uwe Kleine-König

2010-01-13 08:49 UTC #1 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different defines of static(void)

Hello,

Quoted 6 lines · 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

Quoted 7 lines

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

Signature · 2 lines

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

Uwe Kleine-König

2010-01-13 14:26 UTC #2 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different defines of static(void)

Hello,

Quoted 11 lines · Uwe Kleine-K?nig

Quoted 10 lines · 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.

Quoted 6 lines

Quoted 3 lines
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

Signature · 2 lines

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

Tony Lindgren

author

2010-01-13 17:14 UTC #3 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different defines of static(void)

Quoted 20 lines · Uwe Kleine-K?nig

Hello,
Quoted 17 lines · Uwe Kleine-K?nig
Quoted 6 lines · 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
Quoted 7 lines
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?

Quoted 2 lines

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

H Hartley Sweeten

2010-01-13 18:11 UTC #4 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different definesof static(void)

In reply to Uwe Kleine-König

Quoted 6 lines · Uwe Kleine-K?nig

Quoted 3 lines
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

Uwe Kleine-König

2010-01-14 08:29 UTC #5 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different definesof static(void)

Hello Hartley,

Quoted 22 lines · H Hartley Sweeten

Quoted 6 lines · Uwe Kleine-K?nig
Quoted 3 lines
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?

Quoted 3 lines

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

Signature · 2 lines

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

Russell King - ARM Linux

2010-01-14 09:15 UTC #6 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different definesof static(void)

Quoted 2 lines · Uwe Kleine-K?nig

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.

Quoted 1 line

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.

Tony Lindgren

author

2010-01-14 19:38 UTC #7 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different definesof static(void)

Quoted 17 lines · Russell King - ARM Linux

Quoted 2 lines · Uwe Kleine-K?nig
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.
Quoted 1 line
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.

Quoted 4 lines

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

Uwe Kleine-König

2010-01-18 10:44 UTC #8 lore raw

PATCH 1/1 ARM: Fix uncompress code compile for different definesof static(void)

Hello Russell,

Quoted 18 lines · Tony Lindgren

Quoted 17 lines · Russell King - ARM Linux
Quoted 2 lines · Uwe Kleine-K?nig
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.
Quoted 1 line
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.

Quoted 7 lines

Quoted 4 lines
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

Signature · 2 lines

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