| Message ID | 1349441933-22840-1-git-send-email-monstr@monstr.eu |
|---|---|
| State | Accepted |
| Delegated to: | Michal Simek |
| Headers | show |
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
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;
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
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
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;
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(-)