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

login
register
mail settings
Submitter Michal Simek
Date Oct. 5, 2012, 12:58 p.m.
Message ID <1349441933-22840-1-git-send-email-monstr@monstr.eu>
Download mbox | patch
Permalink /patch/189472/
State Accepted
Delegated to: Michal Simek
Headers show

Comments

Michal Simek - Oct. 5, 2012, 12:58 p.m.
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(-)
Marek Vasut - Oct. 5, 2012, 4:48 p.m.
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
Stephan Linz - Oct. 11, 2012, 10:31 p.m.
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;
Michal Simek - Nov. 7, 2012, 4:08 p.m.
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
Marek Vasut - Nov. 8, 2012, 1:30 a.m.
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
Michal Simek - Nov. 8, 2012, 7:57 a.m.
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

Patch

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;