Skip to contentSkip to conversation

microblaze: Fix compilation warning in ext2_find_next_zero_bit

PATCH New Michal Simek 3 participants 13 messages Oct 5, 2012 – Nov 8, 2012

Subsystems (2)
Microblaze architecture Bitops API
lore
Share & tools
lore

Michal Simek

author maintainer
#0 lore raw

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 <[email protected]>
---
 arch/microblaze/include/asm/bitops.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/microblaze/include/asm/bitops.h +2 -1
index e8c835f..eafa2b5 100644
@@ -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;
Signature · 1 line
1.7.0.4
author maintainer
#1 lore raw

PATCH 2/4 microblaze: Fix byteorder for microblaze

Just remove ancient code.

Signed-off-by Michal Simek <[email protected]>
---
 arch/microblaze/include/asm/byteorder.h |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)
arch/microblaze/include/asm/byteorder.h +0 -23
index b2757a4..f3a471d 100644
@@ -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__
Signature · 1 line
1.7.0.4
#3 lore raw

PATCH 2/4 microblaze: Fix byteorder for microblaze

In reply to Michal Simek

Am Freitag, den 05.10.2012, 14:58 +0200 schrieb Michal Simek:

Quoted 28 lines

Just remove ancient code.

Signed-off-by Michal Simek <[email protected]>
---
 arch/microblaze/include/asm/byteorder.h |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)
arch/microblaze/include/asm/byteorder.h +0 -13
index b2757a4..f3a471d 100644
@@ -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;

arch/microblaze/include/asm/byteorder.h line 45

Quoted 10 lines
arch/microblaze/include/asm/byteorder.h +0 -10
-
-	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 <[email protected]>
Quoted 4 lines
arch/microblaze/include/asm/byteorder.h +0 -0
 #if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
 #  define __BYTEORDER_HAS_U64__
 #  define __SWAB_64_THRU_32__
author maintainer
#4 lore raw

PATCH 3/4 microblaze: Flush caches before enabling them

In reply to Michal Simek

Flushing caches is necessary because of soft reset which doesn't clear caches.

Signed-off-by Michal Simek <[email protected]>
---
 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(-)
arch/microblaze/cpu/cache.c +0 -5
index d258a69..ce066b9 100644
@@ -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);
 }
arch/microblaze/cpu/start.S +6 -0
index 8564c4e..3da711d 100644
@@ -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
arch/microblaze/lib/bootm.c +0 -5
index 95cee50..66d21f4 100644
@@ -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):
include/configs/microblaze-generic.h +4 -0
index 721cd90..eed38c1 100644
@@ -287,6 +287,10 @@
 # undef CONFIG_DCACHE
 #endif
 
+#ifndef XILINX_DCACHE_BYTE_SIZE
+#define XILINX_DCACHE_BYTE_SIZE	32768
+#endif
+
 /*
  * BOOTP options
  */
Signature · 1 line
1.7.0.4
author maintainer
#6 lore raw

PATCH 4/4 microblaze: Fix compilation failure because of missing libdts

In reply to Michal Simek

Protect dts/libdts.o by CONFIG_OF_EMBED.

Signed-off-by Michal Simek <[email protected]>
---
 arch/microblaze/cpu/u-boot.lds |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
arch/microblaze/cpu/u-boot.lds +2 -0
index d033a28..349d36a 100644
@@ -45,7 +45,9 @@ SECTIONS
 	.data ALIGN(0x4):
 	{
 		__data_start = .;
+#ifdef CONFIG_OF_EMBED
 		dts/libdts.o (.data)
+#endif
 		*(.data)
 		__data_end = .;
 	}
Signature · 1 line
1.7.0.4
#7 lore raw

PATCH 4/4 microblaze: Fix compilation failure because of missing libdts

Dear Michal Simek,

Quoted 1 line

Protect dts/libdts.o by CONFIG_OF_EMBED.

Better commit message would really help ... esp. if you could explain the problem.

Quoted 10 lines
Signed-off-by Michal Simek <[email protected]>
---
 arch/microblaze/cpu/u-boot.lds |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
(quoted diff) +0 -0

b/arch/microblaze/cpu/u-boot.lds index d033a28..349d36a 100644

arch/microblaze/cpu/u-boot.lds SECTIONS · line 53

Quoted 9 lines
arch/microblaze/cpu/u-boot.lds +2 -0
@@ -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

#8 lore raw

PATCH 1/4 microblaze: Fix compilation warning in ext2_find_next_zero_bit

In reply to Michal Simek

Dear Michal Simek,

Quoted 23 lines

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 <[email protected]>
---
 arch/microblaze/include/asm/bitops.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644

arch/microblaze/include/asm/bitops.h +0 -1
@@ -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,

arch/microblaze/include/asm/bitops.h extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) · line 326

Quoted 4 lines
arch/microblaze/include/asm/bitops.h +1 -0
+				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

author maintainer
#9 lore raw

PATCH 1/4 microblaze: Fix compilation warning in ext2_find_next_zero_bit

Quoted 22 lines · Marek Vasut

Dear Michal Simek,

Quoted 20 lines

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 <[email protected]>
---
  arch/microblaze/include/asm/bitops.h |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644

arch/microblaze/include/asm/bitops.h +0 -0
@@ -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)
Quoted 10 lines

arch/microblaze/include/asm/bitops.h extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) · line 326

Quoted 7 lines
arch/microblaze/include/asm/bitops.h +0 -1
-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,

arch/microblaze/include/asm/bitops.h +1 -0
+				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,

Thanks,
Michal

Signature · 4 lines
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
#10 lore raw

PATCH 1/4 microblaze: Fix compilation warning in ext2_find_next_zero_bit

Dear Michal Simek,

Quoted 53 lines · Marek Vasut
Quoted 29 lines

Dear Michal Simek,

Quoted 27 lines

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

---

  arch/microblaze/include/asm/bitops.h |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644

arch/microblaze/include/asm/bitops.h +0 -1
@@ -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,

arch/microblaze/include/asm/bitops.h +1 -0
+				unsigned long size, unsigned long offset)
Quoted 7 lines
Quoted 4 lines
  {
  
  	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.

Quoted 9 lines
$ 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

author maintainer
#11 lore raw

PATCH 1/4 microblaze: Fix compilation warning in ext2_find_next_zero_bit

Quoted 65 lines · Marek Vasut

Dear Michal Simek,

Quoted 53 lines · Marek Vasut
Quoted 29 lines

Dear Michal Simek,

Quoted 27 lines

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

---

   arch/microblaze/include/asm/bitops.h |    3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/arch/microblaze/include/asm/bitops.h index e8c835f..eafa2b5 100644

arch/microblaze/include/asm/bitops.h +0 -1
@@ -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,

arch/microblaze/include/asm/bitops.h +1 -0
+				unsigned long size, unsigned long offset)
Quoted 7 lines
Quoted 4 lines
   {

   	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.

Quoted 9 lines
$ 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,
Quoted 2 lines

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

Signature · 4 lines
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
#12 lore raw

PATCH 1/4 microblaze: Fix compilation warning in ext2_find_next_zero_bit

In reply to Michal Simek

Am Freitag, den 05.10.2012, 14:58 +0200 schrieb Michal Simek:

Quoted 17 lines

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 <[email protected]>
---
 arch/microblaze/include/asm/bitops.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/microblaze/include/asm/bitops.h +0 -0
index e8c835f..eafa2b5 100644

arch/microblaze/include/asm/bitops.h extern __inline__ int ext2_test_bit(int nr, const volatile void * addr) · line 323

Quoted 6 lines
arch/microblaze/include/asm/bitops.h +2 -1
@@ -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 <[email protected]>
Quoted 3 lines
arch/microblaze/include/asm/bitops.h +0 -0
 {
 	unsigned long *p = ((unsigned long *) addr) + (offset >> 5);
 	unsigned long result = offset & ~31UL;