[U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit

ext2_find_next_zero_bit must be also static if __swab32 is also static.
Warning: include/asm/bitops.h:369:22: warning: '__fswab32' is static but used in inline function 'ext2_find_next_zero_bit' which is not static [enabled by default]
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/include/asm/bitops.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/microblaze/include/asm/bitops.h b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644 --- a/arch/microblaze/include/asm/bitops.h +++ b/arch/microblaze/include/asm/bitops.h @@ -319,7 +319,8 @@ extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) #define ext2_find_first_zero_bit(addr, size) \ ext2_find_next_zero_bit((addr), (size), 0)
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) +static inline unsigned long ext2_find_next_zero_bit(void *addr, + unsigned long size, unsigned long offset) { unsigned long *p = ((unsigned long *) addr) + (offset >> 5); unsigned long result = offset & ~31UL;

Just remove ancient code.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/include/asm/byteorder.h | 23 ----------------------- 1 files changed, 0 insertions(+), 23 deletions(-)
diff --git a/arch/microblaze/include/asm/byteorder.h b/arch/microblaze/include/asm/byteorder.h index b2757a4..f3a471d 100644 --- a/arch/microblaze/include/asm/byteorder.h +++ b/arch/microblaze/include/asm/byteorder.h @@ -20,29 +20,6 @@
#ifdef __GNUC__
-/* This is effectively a dupe of the arch-independent byteswap - code in include/linux/byteorder/swab.h, however we force a cast - of the result up to 32 bits. This in turn forces the compiler - to explicitly clear the high 16 bits, which it wasn't doing otherwise. - - I think this is a symptom of a bug in mb-gcc. JW 20040303 -*/ - - -static __inline__ __u16 ___arch__swab16 (__u16 half_word) -{ - /* 32 bit temp to cast result, forcing clearing of high word */ - __u32 temp; - - temp = ((half_word & 0x00FFU) << 8) | ((half_word & 0xFF00U) >> 8); - - return (__u16) temp; -} - -#define __arch__swab16(x) ___arch__swab16(x) - -/* Microblaze has no arch-specific endian conversion insns */ - #if !defined(__STRICT_ANSI__) || defined(__KERNEL__) # define __BYTEORDER_HAS_U64__ # define __SWAB_64_THRU_32__

Dear Michal Simek,
Just remove ancient code.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/byteorder.h | 23 ----------------------- 1 files changed, 0 insertions(+), 23 deletions(-)
[...]
Reviewed-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut

Am Freitag, den 05.10.2012, 14:58 +0200 schrieb Michal Simek:
Just remove ancient code.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/byteorder.h | 23 ----------------------- 1 files changed, 0 insertions(+), 23 deletions(-)
diff --git a/arch/microblaze/include/asm/byteorder.h b/arch/microblaze/include/asm/byteorder.h index b2757a4..f3a471d 100644 --- a/arch/microblaze/include/asm/byteorder.h +++ b/arch/microblaze/include/asm/byteorder.h @@ -20,29 +20,6 @@
#ifdef __GNUC__
-/* This is effectively a dupe of the arch-independent byteswap
- code in include/linux/byteorder/swab.h, however we force a cast
- of the result up to 32 bits. This in turn forces the compiler
- to explicitly clear the high 16 bits, which it wasn't doing otherwise.
- I think this is a symptom of a bug in mb-gcc. JW 20040303
-*/
-static __inline__ __u16 ___arch__swab16 (__u16 half_word) -{
- /* 32 bit temp to cast result, forcing clearing of high word */
- __u32 temp;
- temp = ((half_word & 0x00FFU) << 8) | ((half_word & 0xFF00U) >> 8);
- return (__u16) temp;
-}
-#define __arch__swab16(x) ___arch__swab16(x)
-/* Microblaze has no arch-specific endian conversion insns */
Acked-by: Stephan Linz linz@li-pro.net
#if !defined(__STRICT_ANSI__) || defined(__KERNEL__) # define __BYTEORDER_HAS_U64__ # define __SWAB_64_THRU_32__

Flushing caches is necessary because of soft reset which doesn't clear caches.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/cache.c | 5 ----- arch/microblaze/cpu/start.S | 6 ++++++ arch/microblaze/lib/bootm.c | 5 ----- include/configs/microblaze-generic.h | 4 ++++ 4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/microblaze/cpu/cache.c b/arch/microblaze/cpu/cache.c index d258a69..ce066b9 100644 --- a/arch/microblaze/cpu/cache.c +++ b/arch/microblaze/cpu/cache.c @@ -61,12 +61,7 @@ void dcache_enable (void) {
void dcache_disable(void) { #ifdef XILINX_USE_DCACHE -#ifdef XILINX_DCACHE_BYTE_SIZE flush_cache(0, XILINX_DCACHE_BYTE_SIZE); -#else -#warning please rebuild BSPs and update configuration - flush_cache(0, 32768); -#endif #endif MSRCLR(0x80); } diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S index 8564c4e..3da711d 100644 --- a/arch/microblaze/cpu/start.S +++ b/arch/microblaze/cpu/start.S @@ -132,6 +132,12 @@ _start: rsubi r8, r10, 0x26 sh r6, r0, r8
+ /* Flush cache before enable cache */ + addik r5, r0, 0 + addik r6, r0, XILINX_DCACHE_BYTE_SIZE +flush: bralid r15, flush_cache + nop + /* enable instruction and data cache */ mfs r12, rmsr ori r12, r12, 0xa0 diff --git a/arch/microblaze/lib/bootm.c b/arch/microblaze/lib/bootm.c index 95cee50..66d21f4 100644 --- a/arch/microblaze/lib/bootm.c +++ b/arch/microblaze/lib/bootm.c @@ -70,12 +70,7 @@ int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima #endif
#ifdef XILINX_USE_DCACHE -#ifdef XILINX_DCACHE_BYTE_SIZE flush_cache(0, XILINX_DCACHE_BYTE_SIZE); -#else -#warning please rebuild BSPs and update configuration - flush_cache(0, 32768); -#endif #endif /* * Linux Kernel Parameters (passing device tree): diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h index 721cd90..eed38c1 100644 --- a/include/configs/microblaze-generic.h +++ b/include/configs/microblaze-generic.h @@ -287,6 +287,10 @@ # undef CONFIG_DCACHE #endif
+#ifndef XILINX_DCACHE_BYTE_SIZE +#define XILINX_DCACHE_BYTE_SIZE 32768 +#endif + /* * BOOTP options */

Dear Michal Simek,
Flushing caches is necessary because of soft reset which doesn't clear caches.
Signed-off-by: Michal Simek monstr@monstr.eu
[...]
Can the flushes not remain in the C code?
Otherwise
Reviewed-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut

Protect dts/libdts.o by CONFIG_OF_EMBED.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/u-boot.lds | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/u-boot.lds b/arch/microblaze/cpu/u-boot.lds index d033a28..349d36a 100644 --- a/arch/microblaze/cpu/u-boot.lds +++ b/arch/microblaze/cpu/u-boot.lds @@ -45,7 +45,9 @@ SECTIONS .data ALIGN(0x4): { __data_start = .; +#ifdef CONFIG_OF_EMBED dts/libdts.o (.data) +#endif *(.data) __data_end = .; }

Dear Michal Simek,
Protect dts/libdts.o by CONFIG_OF_EMBED.
Better commit message would really help ... esp. if you could explain the problem.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/u-boot.lds | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/u-boot.lds b/arch/microblaze/cpu/u-boot.lds index d033a28..349d36a 100644 --- a/arch/microblaze/cpu/u-boot.lds +++ b/arch/microblaze/cpu/u-boot.lds @@ -45,7 +45,9 @@ SECTIONS .data ALIGN(0x4): { __data_start = .; +#ifdef CONFIG_OF_EMBED dts/libdts.o (.data) +#endif *(.data) __data_end = .; }
Best regards, Marek Vasut

Dear Michal Simek,
ext2_find_next_zero_bit must be also static if __swab32 is also static.
Warning: include/asm/bitops.h:369:22: warning: '__fswab32' is static but used in inline function 'ext2_find_next_zero_bit' which is not static [enabled by default]
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/bitops.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/microblaze/include/asm/bitops.h b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644 --- a/arch/microblaze/include/asm/bitops.h +++ b/arch/microblaze/include/asm/bitops.h @@ -319,7 +319,8 @@ extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) #define ext2_find_first_zero_bit(addr, size) \ ext2_find_next_zero_bit((addr), (size), 0)
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) +static inline unsigned long ext2_find_next_zero_bit(void *addr,
unsigned long size, unsigned long offset)
{ unsigned long *p = ((unsigned long *) addr) + (offset >> 5); unsigned long result = offset & ~31UL;
I'd rather see it done the other way -- drop the inline and let compiler decide. What are the size penalties ?
Best regards, Marek Vasut

On 10/05/2012 06:48 PM, Marek Vasut wrote:
Dear Michal Simek,
ext2_find_next_zero_bit must be also static if __swab32 is also static.
Warning: include/asm/bitops.h:369:22: warning: '__fswab32' is static but used in inline function 'ext2_find_next_zero_bit' which is not static [enabled by default]
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/bitops.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/microblaze/include/asm/bitops.h b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644 --- a/arch/microblaze/include/asm/bitops.h +++ b/arch/microblaze/include/asm/bitops.h @@ -319,7 +319,8 @@ extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) #define ext2_find_first_zero_bit(addr, size) \ ext2_find_next_zero_bit((addr), (size), 0)
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) +static inline unsigned long ext2_find_next_zero_bit(void *addr,
{ unsigned long *p = ((unsigned long *) addr) + (offset >> 5); unsigned long result = offset & ~31UL;unsigned long size, unsigned long offset)
I'd rather see it done the other way -- drop the inline and let compiler decide. What are the size penalties ?
With inline text data bss dec hex filename 361914 14698 232344 608956 94abc ./u-boot
without inline text data bss dec hex filename 361922 14698 232368 608988 94adc ./u-boot
But the problem is that I can see a lot of warnings that this function is unused. u-boot/include/asm/bitops.h:322:22: warning: 'ext2_find_next_zero_bit' defined but not used [-Wunused-function]
FYI: I have just grepped source tree and I see that the same solution is used by blackfin/mips and powerpc.
$ grep -rn "ext2_find_next_zero_bit" arch/ | grep static arch/microblaze/include/asm/bitops.h:322:static inline unsigned long ext2_find_next_zero_bit(void *addr, arch/blackfin/include/asm/bitops.h:324:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr, arch/mips/include/asm/bitops.h:830:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) arch/powerpc/include/asm/bitops.h:344:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr,
Thanks, Michal

Dear Michal Simek,
On 10/05/2012 06:48 PM, Marek Vasut wrote:
Dear Michal Simek,
ext2_find_next_zero_bit must be also static if __swab32 is also static.
Warning: include/asm/bitops.h:369:22: warning: '__fswab32' is static but used in inline function 'ext2_find_next_zero_bit' which is not static [enabled by default]
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/bitops.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/microblaze/include/asm/bitops.h b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644 --- a/arch/microblaze/include/asm/bitops.h +++ b/arch/microblaze/include/asm/bitops.h @@ -319,7 +319,8 @@ extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) #define ext2_find_first_zero_bit(addr, size) \
ext2_find_next_zero_bit((addr), (size), 0)
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) +static inline unsigned long ext2_find_next_zero_bit(void *addr,
unsigned long size, unsigned long offset)
{
unsigned long *p = ((unsigned long *) addr) + (offset >> 5); unsigned long result = offset & ~31UL;
I'd rather see it done the other way -- drop the inline and let compiler decide. What are the size penalties ?
With inline text data bss dec hex filename 361914 14698 232344 608956 94abc ./u-boot
without inline text data bss dec hex filename 361922 14698 232368 608988 94adc ./u-boot
But the problem is that I can see a lot of warnings that this function is unused. u-boot/include/asm/bitops.h:322:22: warning: 'ext2_find_next_zero_bit' defined but not used [-Wunused-function]
FYI: I have just grepped source tree and I see that the same solution is used by blackfin/mips and powerpc.
$ grep -rn "ext2_find_next_zero_bit" arch/ | grep static arch/microblaze/include/asm/bitops.h:322:static inline unsigned long ext2_find_next_zero_bit(void *addr, arch/blackfin/include/asm/bitops.h:324:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr, arch/mips/include/asm/bitops.h:830:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) arch/powerpc/include/asm/bitops.h:344:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr,
DAMN :-( Maybe we should focus on --gc-sections for whole u-boot. Anyway, I'd say apply this and then start working on the --gc-sections.
Best regards, Marek Vasut

On 11/08/2012 02:30 AM, Marek Vasut wrote:
Dear Michal Simek,
On 10/05/2012 06:48 PM, Marek Vasut wrote:
Dear Michal Simek,
ext2_find_next_zero_bit must be also static if __swab32 is also static.
Warning: include/asm/bitops.h:369:22: warning: '__fswab32' is static but used in inline function 'ext2_find_next_zero_bit' which is not static [enabled by default]
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/bitops.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/microblaze/include/asm/bitops.h b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644 --- a/arch/microblaze/include/asm/bitops.h +++ b/arch/microblaze/include/asm/bitops.h @@ -319,7 +319,8 @@ extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) #define ext2_find_first_zero_bit(addr, size) \
ext2_find_next_zero_bit((addr), (size), 0)
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) +static inline unsigned long ext2_find_next_zero_bit(void *addr,
unsigned long size, unsigned long offset)
{
unsigned long *p = ((unsigned long *) addr) + (offset >> 5); unsigned long result = offset & ~31UL;
I'd rather see it done the other way -- drop the inline and let compiler decide. What are the size penalties ?
With inline text data bss dec hex filename 361914 14698 232344 608956 94abc ./u-boot
without inline text data bss dec hex filename 361922 14698 232368 608988 94adc ./u-boot
But the problem is that I can see a lot of warnings that this function is unused. u-boot/include/asm/bitops.h:322:22: warning: 'ext2_find_next_zero_bit' defined but not used [-Wunused-function]
FYI: I have just grepped source tree and I see that the same solution is used by blackfin/mips and powerpc.
$ grep -rn "ext2_find_next_zero_bit" arch/ | grep static arch/microblaze/include/asm/bitops.h:322:static inline unsigned long ext2_find_next_zero_bit(void *addr, arch/blackfin/include/asm/bitops.h:324:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr, arch/mips/include/asm/bitops.h:830:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) arch/powerpc/include/asm/bitops.h:344:static __inline__ unsigned long ext2_find_next_zero_bit(void *addr,
DAMN :-( Maybe we should focus on --gc-sections for whole u-boot. Anyway, I'd say apply this and then start working on the --gc-sections.
I have passed this to our toolchain guy.
Thanks, Michal

Am Freitag, den 05.10.2012, 14:58 +0200 schrieb Michal Simek:
ext2_find_next_zero_bit must be also static if __swab32 is also static.
Warning: include/asm/bitops.h:369:22: warning: '__fswab32' is static but used in inline function 'ext2_find_next_zero_bit' which is not static [enabled by default]
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/include/asm/bitops.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/microblaze/include/asm/bitops.h b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644 --- a/arch/microblaze/include/asm/bitops.h +++ b/arch/microblaze/include/asm/bitops.h @@ -319,7 +319,8 @@ extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) #define ext2_find_first_zero_bit(addr, size) \ ext2_find_next_zero_bit((addr), (size), 0)
-extern __inline__ unsigned long ext2_find_next_zero_bit(void *addr, unsigned long size, unsigned long offset) +static inline unsigned long ext2_find_next_zero_bit(void *addr,
unsigned long size, unsigned long offset)
Acked-by: Stephan Linz linz@li-pro.net
{ unsigned long *p = ((unsigned long *) addr) + (offset >> 5); unsigned long result = offset & ~31UL;
participants (3)
-
Marek Vasut
-
Michal Simek
-
Stephan Linz