All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit
@ 2012-10-05 12:58 Michal Simek
  2012-10-05 12:58 ` [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze Michal Simek
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Michal Simek @ 2012-10-05 12:58 UTC (permalink / raw)
  To: u-boot

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;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze
  2012-10-05 12:58 [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Michal Simek
@ 2012-10-05 12:58 ` Michal Simek
  2012-10-05 16:48   ` Marek Vasut
  2012-10-11 22:31   ` Stephan Linz
  2012-10-05 12:58 ` [U-Boot] [PATCH 3/4] microblaze: Flush caches before enabling them Michal Simek
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Michal Simek @ 2012-10-05 12:58 UTC (permalink / raw)
  To: u-boot

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__
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 3/4] microblaze: Flush caches before enabling them
  2012-10-05 12:58 [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Michal Simek
  2012-10-05 12:58 ` [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze Michal Simek
@ 2012-10-05 12:58 ` Michal Simek
  2012-10-05 16:50   ` Marek Vasut
  2012-10-05 12:58 ` [U-Boot] [PATCH 4/4] microblaze: Fix compilation failure because of missing libdts Michal Simek
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Michal Simek @ 2012-10-05 12:58 UTC (permalink / raw)
  To: u-boot

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
  */
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 4/4] microblaze: Fix compilation failure because of missing libdts
  2012-10-05 12:58 [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Michal Simek
  2012-10-05 12:58 ` [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze Michal Simek
  2012-10-05 12:58 ` [U-Boot] [PATCH 3/4] microblaze: Flush caches before enabling them Michal Simek
@ 2012-10-05 12:58 ` Michal Simek
  2012-10-05 16:51   ` Marek Vasut
  2012-10-05 16:48 ` [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Marek Vasut
  2012-10-11 22:31 ` Stephan Linz
  4 siblings, 1 reply; 13+ messages in thread
From: Michal Simek @ 2012-10-05 12:58 UTC (permalink / raw)
  To: u-boot

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 = .;
 	}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit
  2012-10-05 12:58 [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Michal Simek
                   ` (2 preceding siblings ...)
  2012-10-05 12:58 ` [U-Boot] [PATCH 4/4] microblaze: Fix compilation failure because of missing libdts Michal Simek
@ 2012-10-05 16:48 ` Marek Vasut
  2012-11-07 16:08   ` Michal Simek
  2012-10-11 22:31 ` Stephan Linz
  4 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2012-10-05 16:48 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze
  2012-10-05 12:58 ` [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze Michal Simek
@ 2012-10-05 16:48   ` Marek Vasut
  2012-10-11 22:31   ` Stephan Linz
  1 sibling, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2012-10-05 16:48 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 3/4] microblaze: Flush caches before enabling them
  2012-10-05 12:58 ` [U-Boot] [PATCH 3/4] microblaze: Flush caches before enabling them Michal Simek
@ 2012-10-05 16:50   ` Marek Vasut
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2012-10-05 16:50 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 4/4] microblaze: Fix compilation failure because of missing libdts
  2012-10-05 12:58 ` [U-Boot] [PATCH 4/4] microblaze: Fix compilation failure because of missing libdts Michal Simek
@ 2012-10-05 16:51   ` Marek Vasut
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2012-10-05 16:51 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit
  2012-10-05 12:58 [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Michal Simek
                   ` (3 preceding siblings ...)
  2012-10-05 16:48 ` [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Marek Vasut
@ 2012-10-11 22:31 ` Stephan Linz
  4 siblings, 0 replies; 13+ messages in thread
From: Stephan Linz @ 2012-10-11 22:31 UTC (permalink / raw)
  To: u-boot

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;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze
  2012-10-05 12:58 ` [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze Michal Simek
  2012-10-05 16:48   ` Marek Vasut
@ 2012-10-11 22:31   ` Stephan Linz
  1 sibling, 0 replies; 13+ messages in thread
From: Stephan Linz @ 2012-10-11 22:31 UTC (permalink / raw)
  To: u-boot

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__

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit
  2012-10-05 16:48 ` [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Marek Vasut
@ 2012-11-07 16:08   ` Michal Simek
  2012-11-08  1:30     ` Marek Vasut
  0 siblings, 1 reply; 13+ messages in thread
From: Michal Simek @ 2012-11-07 16:08 UTC (permalink / raw)
  To: u-boot

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,

Thanks,
Michal

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit
  2012-11-07 16:08   ` Michal Simek
@ 2012-11-08  1:30     ` Marek Vasut
  2012-11-08  7:57       ` Michal Simek
  0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2012-11-08  1:30 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit
  2012-11-08  1:30     ` Marek Vasut
@ 2012-11-08  7:57       ` Michal Simek
  0 siblings, 0 replies; 13+ messages in thread
From: Michal Simek @ 2012-11-08  7:57 UTC (permalink / raw)
  To: u-boot

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

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-11-08  7:57 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-05 12:58 [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Michal Simek
2012-10-05 12:58 ` [U-Boot] [PATCH 2/4] microblaze: Fix byteorder for microblaze Michal Simek
2012-10-05 16:48   ` Marek Vasut
2012-10-11 22:31   ` Stephan Linz
2012-10-05 12:58 ` [U-Boot] [PATCH 3/4] microblaze: Flush caches before enabling them Michal Simek
2012-10-05 16:50   ` Marek Vasut
2012-10-05 12:58 ` [U-Boot] [PATCH 4/4] microblaze: Fix compilation failure because of missing libdts Michal Simek
2012-10-05 16:51   ` Marek Vasut
2012-10-05 16:48 ` [U-Boot] [PATCH 1/4] microblaze: Fix compilation warning in ext2_find_next_zero_bit Marek Vasut
2012-11-07 16:08   ` Michal Simek
2012-11-08  1:30     ` Marek Vasut
2012-11-08  7:57       ` Michal Simek
2012-10-11 22:31 ` Stephan Linz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.