[U-Boot] [PATCH] Correct ffs/fls regression for PowerPC etc

Stefan Roese
2009-09-17T12:56:33Z

Hi Simon,

On Thursday 17 September 2009 14:39:40 Simon Kagstrom wrote:
> Correct ffs/fls regression for PowerPC etc
>
> Commits
>
> 02f99901ed1c9d828e3ea117f94ce2264bf8389e
> 52d61227b66d4099b39c8309ab37cb67ee09a405
>
> introduced a regression where platform-specific ffs/fls implementations
> were defined away. This patch corrects that by using PLATFORM_xxx
> instead of the name itself.

Thanks. I tested it on my faulting ppc4xx platform (sequoia). And the problem
is gone now, so thanks.

But I've spotted a problem with other platforms in this patch. See below...

> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> I've runtime tested on ARM (OpenRD base) and build-tested on MPC8536DS,
> where __ilog2_u64 now at least is non-empty. It would be nice if (at
> least!) some PowerPC people could test the patch.
>
> include/asm-arm/bitops.h | 8 --------
> include/asm-blackfin/bitops.h | 10 +---------
> include/asm-i386/bitops.h | 2 +-
> include/asm-m68k/bitops.h | 2 +-
> include/asm-microblaze/bitops.h | 3 ++-
> include/asm-mips/bitops.h | 13 +------------
> include/asm-nios/bitops.h | 2 +-
> include/asm-nios2/bitops.h | 2 +-
> include/asm-ppc/bitops.h | 4 ++--
> include/asm-sh/bitops.h | 2 +-
> include/linux/bitops.h | 8 ++++----
> 11 files changed, 15 insertions(+), 41 deletions(-)
>
> diff --git a/include/asm-arm/bitops.h b/include/asm-arm/bitops.h
> index 3c7b00c..270f163 100644
> --- a/include/asm-arm/bitops.h
> +++ b/include/asm-arm/bitops.h
> @@ -125,14 +125,6 @@ static inline unsigned long ffz(unsigned long word)
> }
>
> /*
> - * ffs: find first bit set. This is defined the same way as
> - * the libc and compiler builtin ffs routines, therefore
> - * differs in spirit from the above ffz (man ffs).
> - */
> -
> -#define ffs(x) generic_ffs(x)
> -
> -/*
> * hweightN: returns the hamming weight (i.e. the number
> * of bits set) of a N-bit word
> */
> diff --git a/include/asm-blackfin/bitops.h b/include/asm-blackfin/bitops.h
> index cc3685d..f469f1c 100644
> --- a/include/asm-blackfin/bitops.h
> +++ b/include/asm-blackfin/bitops.h
> @@ -79,7 +79,7 @@ static __inline__ void __set_bit(int nr, volatile void
> *addr) mask = 1 << (nr & 0x1f);
> *a |= mask;
> }
> -#define __set_bit
> +#define PLATFORM__set_bit
>
> /*
> * clear_bit() doesn't provide any barrier for the compiler.
> @@ -270,14 +270,6 @@ static __inline__ int find_next_zero_bit(void *addr,
> int size, int offset) }
>
> /*
> - * ffs: find first bit set. This is defined the same way as
> - * the libc and compiler builtin ffs routines, therefore
> - * differs in spirit from the above ffz (man ffs).
> - */
> -
> -#define ffs(x) generic_ffs(x)
> -
> -/*
> * hweightN: returns the hamming weight (i.e. the number
> * of bits set) of a N-bit word
> */
> diff --git a/include/asm-i386/bitops.h b/include/asm-i386/bitops.h
> index ac6285a..c7a38f2 100644
> --- a/include/asm-i386/bitops.h
> +++ b/include/asm-i386/bitops.h
> @@ -349,7 +349,7 @@ static __inline__ int ffs(int x)
> "1:" : "=r" (r) : "g" (x));
> return r+1;
> }
> -#define ffs
> +#define PLATFORM_FFS

Here you define "FFS" in upper case.

> /**
> * hweightN - returns the hamming weight of a N-bit word
> diff --git a/include/asm-m68k/bitops.h b/include/asm-m68k/bitops.h
> index e0c35fa..ad971b4 100644
> --- a/include/asm-m68k/bitops.h
> +++ b/include/asm-m68k/bitops.h
> @@ -51,7 +51,7 @@ extern __inline__ int ffs(int x)
> return r;
> }
> #define __ffs(x) (ffs(x) - 1)
> -#define ffs
> +#define PLATFORM_FFS

And here again.

> #endif /* __KERNEL__ */
>
> diff --git a/include/asm-microblaze/bitops.h
> b/include/asm-microblaze/bitops.h index aac9061..3b39e16 100644
> --- a/include/asm-microblaze/bitops.h
> +++ b/include/asm-microblaze/bitops.h
> @@ -23,6 +23,7 @@ extern void __set_bit(int nr, volatile void * addr);
>
> extern void clear_bit(int nr, volatile void * addr);
> #define __clear_bit(nr, addr) clear_bit(nr, addr)
> +#define PLATFORM__clear_bit
>
> extern void change_bit(int nr, volatile void * addr);
> extern void __change_bit(int nr, volatile void * addr);
> @@ -75,7 +76,7 @@ extern __inline__ void __set_bit(int nr, volatile void *
> addr) mask = 1 << (nr & 0x1f);
> *a |= mask;
> }
> -#define __set_bit
> +#define PLATFORM__set_bit
>
> /*
> * clear_bit() doesn't provide any barrier for the compiler.
> diff --git a/include/asm-mips/bitops.h b/include/asm-mips/bitops.h
> index 0c07b68..18dee51 100644
> --- a/include/asm-mips/bitops.h
> +++ b/include/asm-mips/bitops.h
> @@ -90,7 +90,7 @@ static __inline__ void __set_bit(int nr, volatile void *
> addr)
>
> *m |= 1UL << (nr & 31);
> }
> -#define __set_bit
> +#define PLATFORM__set_bit
>
> /*
> * clear_bit - Clears a bit in memory
> @@ -706,17 +706,6 @@ static __inline__ unsigned long ffz(unsigned long
> word)
>
> #ifdef __KERNEL__
>
> -/**
> - * ffs - find first bit set
> - * @x: the word to search
> - *
> - * This is defined the same way as
> - * the libc and compiler builtin ffs routines, therefore
> - * differs in spirit from the above ffz (man ffs).
> - */
> -
> -#define ffs(x) generic_ffs(x)
> -
> /*
> * hweightN - returns the hamming weight of a N-bit word
> * @x: the word to weigh
> diff --git a/include/asm-nios/bitops.h b/include/asm-nios/bitops.h
> index 8315fb7..2047b7c 100644
> --- a/include/asm-nios/bitops.h
> +++ b/include/asm-nios/bitops.h
> @@ -33,6 +33,6 @@ extern int test_and_set_bit(int nr, volatile void * a);
> extern int test_and_change_bit(int nr, volatile void * addr);
> extern int test_bit(int nr, volatile void * a);
> extern int ffs(int i);
> -#define ffs
> +#define PLATFORM_ffs

Here it's lower case.

<snip>

> +++ b/include/linux/bitops.h
> @@ -111,19 +111,19 @@ static inline unsigned int generic_hweight8(unsigned
> int w)
>
> /* linux/include/asm-generic/bitops/non-atomic.h */
>
> -#ifndef __set_bit
> +#ifndef PLATFORM__set_bit
> # define __set_bit generic_set_bit
> #endif
>
> -#ifndef __clear_bit
> +#ifndef PLATFORM__clear_bit
> # define __clear_bit generic_clear_bit
> #endif
>
> -#ifndef ffs
> +#ifndef PLATFORM_ffs
> # define ffs generic_ffs
> #endif

And here you check against lower case "_ffs". So the platforms with upper case
"_FFS" are still broken. Please fix and resubmit.

Thanks.

Cheers,
Stefan

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de